feat(connect): graceful wallet lock — example + docs stop teaching the inverted model - #20
Merged
Conversation
…ck build browser, nodejs and backend-auth/frontend point at the sibling sphere-sdk checkout for the duration of this branch. Relative depths differ per package: browser and nodejs are one level down, backend-auth/frontend is two. The last task of this branch restores published pins — a file: link never ships.
Three packages ran vitest with no config anywhere, so it silently fell back to vite.config.ts and the environment was node — no component or hook test could exist. nodejs and backend-auth/frontend had no runner at all, and the only workflow built browser/** without running a single test. browser now runs under jsdom with React Testing Library, guarded by a smoke test that fails HERE rather than inside a hook test when the environment breaks. CI runs tests and typecheck for all three. Also stops git from tracking docs/superpowers — local working artifacts.
The old rule matched /not.connected|timeout|transport|closed|session/i and disconnected on any error whose TEXT mentioned a session — with no notion of 4009 at all. A locked wallet therefore read as a dead connection. Classification now keys on the numeric code and, for 4009, on data.reason, and a helper reads the wallet's protocol MINOR so a dApp can tell a graceful-lock wallet from an old one.
In popup mode wallet:locked destroyed the transport, dropped the client and cleared sessionStorage — the exact opposite of what a graceful lock means. It is now a STATE in every transport mode: the session, the granted permissions and the transport all survive, and requests are answered 4009 until wallet:unlocked. Tearing down here orphaned a host-side session that now outlives the lock, and the next silent autoConnect would reconnect with no prompt at all. A 2.0 wallet means the opposite by the same event name — its removed notifyWalletLocked() pushed the event AND revoked the session, so no wallet:unlocked will ever arrive. walletProtocol tells the two apart.
…ted as the only teardown An unlock is not implicitly the same wallet: the lock screen's "restore from recovery phrase" installs another seed, and origin approvals carry no identity binding. The hook compares the identity the unlock carries against the one the session was approved for, and flags walletChanged rather than silently resuming against a stranger. unlockEpoch bumps only when the SAME wallet came back — read panels use it as a refetch trigger, never to replay an intent. wallet:disconnected is now the one signal that tears the connection down.
Seven call sites each repeated make-client, connect and setState, so every one of them silently ignored ConnectResult.locked — a dApp that connected to an already-locked wallet believed it was live. They now share one handshake() that records the wallet's protocol MINOR and its locked flag. A permanent HOST_READY listener re-handshakes when the wallet page reloads during a lock: the host that comes back has never seen the old sessionId, so without this the dApp stayed dead until the user clicked Connect again.
isWalletLocked had zero consumers and the header hardcoded a green pulsing "Connected". Under a session-preserving lock that badge is a lie the user acts on while every panel errors. The banner also names the two cases a dApp must not conflate: a 2.0 wallet, where a lock really did end the session, and walletChanged, where the unlock returned a DIFFERENT wallet. BalancePanel refetches on unlockEpoch — a READ is safe to re-issue automatically; an intent never is, which is why only query panels take the prop.
wallet:unlocked and wallet:disconnected never appeared in the event log, so the demo could not show the lifecycle it now implements. wallet:locked also stops being red — a lock is a pause; wallet:disconnected is the fatal one.
…ands The only path that exercises the real ConnectHost/ConnectClient pair without a browser — the cross-repo canary for code === 4009 plus data.reason === 'locked', and the answer to "what does a bot do when there is nobody to type a password": it sees a typed error and fails fast. Pins the whole locked decision table: the allow-list is served from the frozen snapshot, everything else is refused, the resume handshake succeeds with locked: true, and onLockedRequest only ever increments a passive counter — no credential surface is implied anywhere.
A 4009 printed as a mystery error string. The CLI now names the lock, keeps the session, and re-issues the LAST QUERY when the same wallet comes back — checking the identity in the wallet:unlocked payload first, because a restore behind the lock screen installs a different seed. Intents are deliberately never resumed: one would move money with no fresh user gesture, at the exact moment the wallet returned.
The catch disconnected on ANY failure. A locked wallet is not a dead connection: the host preserved the session and will push wallet:unlocked on it, so tearing down there threw away a live session and forced a fresh approval for nothing. The two error helpers move into their own module so they can be tested without mounting the app.
Four documents taught precisely what this feature inverts — that wallet:locked means clear your session and reconnect. This repo is what third-party devs copy, so those docs are deliverables, not commentary. The lock chapter is rewritten around the three verbs, the 4009 row and both new events are documented, and the error-handling example classifies by code instead of by message text. Version claims now say the branch builds against the sibling sphere-sdk checkout rather than naming a version that is not published yet.
Companion to the wallet fix. The wallet now defers HOST_READY until a host can actually complete a handshake, which for a locked wallet means "when a human unlocks it". Two changes make the dApp meet that contract: - the HOST_READY listener is ALWAYS armed, never gated on isConnected. Gating it swallowed the announcement that follows an unlock, because by then the attempt had already failed and nothing was listening — leaving the user pressing Connect at a wallet that was ready. Connected -> resume the same session; not connected -> retry the attempt the user already asked for. Consent is never skipped, only the second click. - a retry against an ALREADY-OPEN popup no longer waits for HOST_READY. That window has booted and announced; nothing will announce again, so waiting hung for the full 30 s timeout. Deliberately NOT a remembered "host is ready" bit: readiness is not monotonic — a wallet can lock again with no wire signal at all, since setLocked() pushes wallet:locked only when a session is active — so a cached bit goes stale with nothing to clear it. The handshake is simply attempted, and a host that cannot serve refuses it promptly instead of hanging. An announcement that arrives while an attempt is in flight is ignored: that attempt is already waiting for it, and acting on it would build a second client for the same handshake.
…t the user raise the window Manual testing showed the wallet's badge climbing to "18 requests blocked" from one origin. The cause was ours: isWalletLocked reached only the header and the banner, so ChatPanel kept loading conversations and messages behind the lock screen and every read collected a 4009. This hook is a reference third-party devs copy, so it was teaching exactly the wrong thing. - ChatPanel takes isWalletLocked/unlockEpoch and does not fetch while locked; the unlock is what brings it back. - focusWallet() raises the wallet window, wired to a BUTTON in the banner. Never automatic: a page that grabbed focus on its own would be a nuisance, and the wallet must remain the only thing that decides when a password field appears. - focusWallet() deliberately does NOT reopen a closed popup. Closing it is a real disconnect (the wallet revokes the session on beforeunload), and a fresh window would cold-start locked anyway because the password is memory-only. Reporting false lets the UI say "reconnect" instead of promising a restore it cannot deliver. - CONNECT.md now says which transport to pick by session lifetime: popup for a short bounded flow, iframe for a long-lived one, and why — closing, reloading, cold-starting locked, and one wallet window per dApp are all inherent to the popup.
sphere-extension is not the wallet host anymore — the Sphere popup is — so naming the extension as a third option a dApp should weigh is stale advice in a document third parties copy. The section now covers only the two live transports. The rest of this file still recommends the extension in ~14 places, including "P2 (extension) is the best mode for production". That predates this branch and is a known item from the July example/marketplace audit; it belongs in its own PR rather than being folded into a lock fix.
…uest hits 4009 Pressing "Fetch Balance" against a locked wallet left the user to hunt for the wallet window among their tabs. Raising it is the right reaction — it is the direct consequence of a click the user just made, and it is what a wallet integration should do. The distinction that makes this safe is not "automatic vs button" but "user-initiated vs background". navigator.userActivation.isActive is the browser's own transient-activation flag; it is sampled SYNCHRONOUSLY at the call site, before any await, which is the only moment the gesture is still live. A poller or a subscription callback has no activation and therefore can never steal focus. Where the flag is unavailable (non-Chromium) the answer is "not user-initiated", because that error only costs a window raise, while the opposite lets a timer grab focus. Only a lock refusal qualifies: classifyRequestError(err) === 'locked'. The wallet still decides everything that follows — it raises its own password field; we only make the window visible. The banner button stays as the fallback for an expired or unavailable activation.
…009 to catch The lock paragraph enumerated only the money reads, so it read as if DMs keep working. They do not: the allow-list is four of sixteen methods, and the twelve refused include sphere_resolve and all four DM reads. It also documents the case an audit critic showed the old wording gets wrong, and it is the COMMON one: a wallet that cold-starts locked holds no session, so the handshake itself is refused with an errorless empty response. A dApp following the documented `if (err.code === WALLET_LOCKED)` pattern catches nothing, because there is no code — treat an unexpected rejection as "not ready yet" and let the next HOST_READY retry.
…cks — the wallet
Reported from testing: reloading the dApp page left the wallet asking for its
password again.
It was not a lock. `window.open(url, name)` does not merely return a window that
already has that name — it NAVIGATES it to `url`. A dApp reload starts a fresh JS
context that has lost its window handle, so both the click path and the mount
resume path called window.open WITH A URL and re-navigated a perfectly good wallet
window. Navigating reloads the wallet page, and the password is memory-only, so the
wallet came back locked.
`window.open('', name)` hands back the existing window without touching it. Both
paths now recover the handle that way and resume on it directly — no
waitForHostReady, because a host that is already live announced once when it came
up and does not announce again. Only when there is nothing live to recover is the
window navigated, and it is the SAME window, so a blank one is not left stray.
All five packages move off the local file: link onto the released SDK, and the README/CLAUDE pin claims say the real version again. The interesting part is not the pin. 0.13.0 adds INTENT_OUTCOME_UNKNOWN (4201) — the wallet took the intent and the answer was lost, so the money may or may not have moved — and this repo is what dApp authors copy. It classified 4201 as an ordinary refusal, which is the one reading that costs money: a panel treats "failed" as "let them press Send again", and the user pays twice. - classifyRequestError gains its own 'outcome-unknown' kind, deliberately kept apart from 'other' so a UI cannot flatten the two. The connection is fine, so nothing is torn down and nothing is flagged locked. - CONNECT.md gets a spending section showing the switch, and says plainly that USER_REJECTED is different and IS retryable. - the nodejs lock-gate test asserted the old sphere_subscribe behaviour, which was reverted in the SDK because throwing there silently broke every pre-2.1 dApp.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Example/docs half of the graceful wallet lock. Depends on sphere-sdk PR #695 and pairs with sphere PR #458.
Why this repo matters here
This is what third-party devs copy. Before this branch it taught the exact opposite of the feature:
useWalletConnectdestroyed the transport and clearedsessionStorageonwallet:locked, and four documents said a lock means "clear your session and reconnect".What changes
wallet:lockedsets a flag;wallet:disconnectedis the only teardown. A Connect 2.0 wallet still gets the old teardown — there the same event really did revoke the session, andclient.walletProtocoltells them apart..codeanddata.reason, never by message text. The old rule was/not.connected|timeout|transport|closed|session/i— it disconnected on any error whose text mentioned a session and had no notion of4009.wallet:unlockedis identity-checked. An unlock is not implicitly the same wallet: the lock screen's "restore from recovery phrase" installs another seed. Same wallet →unlockEpochbumps and read panels refetch. Different wallet →walletChanged, and nothing resumes.handshake(), soConnectResult.lockedis honoured everywhere — previously a dApp connecting to an already-locked wallet believed it was live. A permanentHOST_READYlistener re-handshakes after a wallet-page reload during a lock.4009test drives the realConnectHost/ConnectClientpair over an in-process loopback — the cross-repo canary, and the answer to "what does a bot do when nobody can type a password": it fails fast on a typed error. The mock wallet server gainslock/unlock/logout/unavailable/statusstdin commands.Test infrastructure
There was no vitest config anywhere, so vitest silently fell back to
vite.config.tsand ran undernode— no component or hook test could exist.nodejsandbackend-auth/frontendhad no runner at all, and the only workflow builtbrowser/**without running a single test. This branch adds jsdom + React Testing Library, runners for all packages, and a CI job.Verification
Documentation grep gates pass: no
Full disconnect, noextension/iframe only, noresubscribeAll, noSPHERE_CONNECT_VERSION = '2.0'.Not done here, on purpose
Packages point at
file:../../sphere-sdk, and the docs say so rather than naming a version. The published pin and the SDK version number are the owner's call.Fixes added after this PR was opened
The reference hook was polling into a locked wallet.
isWalletLockedreached only the header and the banner, soChatPanelkept loading conversations and messages behind the lock screen and every read collected a 4009 — the wallet's badge climbed to "18 requests blocked" from one origin during testing. This hook is what third-party devs copy, so it was teaching exactly the wrong thing. It now stops fetching while locked and resumes onunlockEpoch.A failed attempt against a locked wallet heals itself. The wallet defers
HOST_READYuntil a host can actually complete a handshake, which for a locked wallet means "when a human unlocks it". TheHOST_READYlistener is therefore always armed, never gated onisConnected— gating it swallowed the announcement that follows an unlock, leaving the user pressing Connect at a wallet that was already ready. A retry against an already-open popup also no longer waits forHOST_READY: that window has booted and announced, so waiting hung for the full 30 s.Deliberately not a remembered "host is ready" bit: readiness is not monotonic — a wallet can lock again with no wire signal at all, since
setLocked()pusheswallet:lockedonly when a session is active — so a cached bit goes stale with nothing to clear it.A user-triggered request that hits 4009 raises the wallet window. The distinction that makes this safe is not "automatic vs button" but user-initiated vs background:
navigator.userActivation.isActiveis sampled synchronously at the call site, before any await, which is the only moment the gesture is still live. A poller has no activation and can never steal focus.focusWallet()deliberately does not reopen a closed popup — closing it is a real disconnect (the wallet revokes the session onbeforeunload) and a fresh window cold-starts locked anyway, so promising a restore would be a lie.CONNECT.mdnow says which transport to pick by session lifetime. Popup for a short bounded flow — closing it disconnects, reloading it re-locks the wallet, it cold-starts locked even when Sphere is unlocked in another tab, and it does not scale to several dApps. Iframe for a long-lived session: one wallet window, many hosts, one unlock, and the badge in the wallet's own chrome.The lock paragraph was under-stated. It named only the money reads, so it read as if DMs keep working. The allow-list is four of sixteen methods; the twelve refused include
sphere_resolveand all four DM reads. It also documents the case the old wording gets wrong, and it is the common one: a wallet that cold-starts locked holds no session, so the handshake itself is refused with an errorless empty response — a generatedcatch (err.code === WALLET_LOCKED)catches nothing, because there is no code.58tests across 9 files inbrowser, plus 18 / 8 / 4 / 25 in the other packages. Typecheck clean everywhere.