fix(frontend): abort cancelled Terra connect and keep header above wallet backdrops (GL-137) #162

Merged
PlasticDigits merged 2 commits from fix/gl-137-cancel-abort-overlay into main 2026-09-02 01:38:24 +00:00
PlasticDigits commented 2026-09-01 07:29:37 +00:00 (Migrated from gitlab.com)

Summary

  • Follow-up to merged !159 for #137. Core Android Chrome connect path is already on main; this hardens remaining completion-path and overlay gaps.
  • Cancel abort: cancelConnection / disconnect bumps a connect epoch so a late connectTerraWallet success cannot set connected. Ghost sessions are disconnected. ConnectionCancelledError is not shown as connectionError.
  • Header-safe backdrops: connected-wallet fixed inset-0 menus (Terra / EVM / Solana) portal to document.body at z-40 (WalletMenuBackdrop) and close on route change (useDismissOnNavigate), so they cannot stack inside the header and cover Connect.
  • Tests: hydrate reset, Cancel-vs-late-WC, pairing Cancel, Simulated wallet hidden when DEV_MODE is false, in-app banner, Android Chrome UA e2e, History navigation dismisses backdrop.
  • Invariants INV-FE-WC-MOBILE-1, docs/frontend.md, QA template 3.11/3.15, and skills/agent-frontend-terra-wallet-mobile.md updated and crosslinked.

Related #137 (kept open until Android 16 Chrome device QA passes)

Test plan

  • Unit tests for Cancel abort, hydrate reset, backdrop portal, pairing Cancel, Simulated hidden in production mock, in-app banner (npm run test:unit on changed files — 27 passed; retested)
  • Typecheck (npx tsc --noEmit)
  • ESLint on changed TS
  • Playwright E2E_UI_ONLY=1 npx playwright test e2e/wallet-connect.spec.ts --workers=5 — 9 passed (retested after backdrop portal)
  • Tested on desktop (device: ___) — desktop extension path unchanged; please confirm Keplr extension
  • Tested on mobile (device: Android 16 Chrome) — still needs device QA (tap CTA → modal → Lunc Dash / Galaxy Station Open+Copy)
  • Wallet tested: ___
  • Screenshots attached (if UI change) — overlay stacking screenshot still useful from a real Android 16 Chrome session

Checklist

  • Only touches packages/frontend/ plus docs/skills/QA for invariants
  • No hardcoded secrets, keys, or addresses
  • Lint passes
  • Unit tests pass (changed files)
## Summary - Follow-up to merged !159 for [#137](https://gitlab.com/PlasticDigits/cl8y-bridge-monorepo/-/issues/137). Core Android Chrome connect path is already on `main`; this hardens remaining completion-path and overlay gaps. - **Cancel abort:** `cancelConnection` / disconnect bumps a connect epoch so a late `connectTerraWallet` success cannot set `connected`. Ghost sessions are disconnected. `ConnectionCancelledError` is not shown as `connectionError`. - **Header-safe backdrops:** connected-wallet `fixed inset-0` menus (Terra / EVM / Solana) portal to `document.body` at `z-40` (`WalletMenuBackdrop`) and close on route change (`useDismissOnNavigate`), so they cannot stack inside the header and cover Connect. - Tests: hydrate reset, Cancel-vs-late-WC, pairing Cancel, Simulated wallet hidden when `DEV_MODE` is false, in-app banner, Android Chrome UA e2e, History navigation dismisses backdrop. - Invariants **INV-FE-WC-MOBILE-1**, `docs/frontend.md`, QA template 3.11/3.15, and `skills/agent-frontend-terra-wallet-mobile.md` updated and crosslinked. Related #137 (kept open until Android 16 Chrome device QA passes) ## Test plan - [x] Unit tests for Cancel abort, hydrate reset, backdrop portal, pairing Cancel, Simulated hidden in production mock, in-app banner (`npm run test:unit` on changed files — 27 passed; retested) - [x] Typecheck (`npx tsc --noEmit`) - [x] ESLint on changed TS - [x] Playwright `E2E_UI_ONLY=1 npx playwright test e2e/wallet-connect.spec.ts --workers=5` — 9 passed (retested after backdrop portal) - [ ] Tested on desktop (device: ___) — desktop extension path unchanged; please confirm Keplr extension - [ ] Tested on mobile (device: Android 16 Chrome) — still needs device QA (tap CTA → modal → Lunc Dash / Galaxy Station Open+Copy) - [ ] Wallet tested: ___ - [ ] Screenshots attached (if UI change) — overlay stacking screenshot still useful from a real Android 16 Chrome session ## Checklist - [x] Only touches `packages/frontend/` plus docs/skills/QA for invariants - [x] No hardcoded secrets, keys, or addresses - [x] Lint passes - [x] Unit tests pass (changed files)
PlasticDigits commented 2026-09-01 07:29:56 +00:00 (Migrated from gitlab.com)

mentioned in issue #137

mentioned in issue #137
PlasticDigits commented 2026-09-01 08:00:22 +00:00 (Migrated from gitlab.com)

RECOMMEND: FIX

Follow-up to !159 for #137. Cancel-abort and header-safe backdrops are real improvements (connect epoch, WalletMenuBackdrop portaled at z-40 under header z-50, useDismissOnNavigate). No prompt injection, XSS, WC-allowlist regression, or Legal-gate skip. Do not close #137 until Android 16 Chrome device QA.

Must-fix

Stale connectTerraWallet success always calls disconnectTerraWallet(), which can kill Retry.

packages/frontend/src/stores/wallet.ts (epoch mismatch after await connectTerraWallet):

if (epoch !== connectEpoch) {
  await disconnectTerraWallet()
  throw new ConnectionCancelledError()
}

connectTerraWallet already wrote into connectedWallets before returning. TerraWalletModal.handleRetry does cancelConnection() then connect() 100ms later. Cosmes KeplrController.disconnect then calls this.wc.disconnect() when the controller map is empty — that drops the shared WalletConnect client under in-flight Retry.

Unit tests only cover Cancel without a second connect() (wallet.test.ts).

Required: skip protocol disconnect when get().connecting is true (a newer connect owns the singleton). Still throw ConnectionCancelledError and do not set({ connected: true }). Add a unit test: connect A → cancelConnection → connect B → resolve A → connected still false, B still running, disconnectTerraWallet not called while B is connecting.

Issue #137

AC This MR
Android 16 Chrome first tap opens modal NOT MET (honestly deferred; Playwright UA ≠ device)
connecting === false on fresh visit MET (applyWalletHydrateReset)
Lunc Dash / Galaxy Open+Copy PARTIAL (!159 pairing; device QA still open)
Keplr not a dead “Not installed” row N/A (!159)
Desktop extension unchanged MET
Legal gate does not swallow Connect MET (TermsGate is transfer-only; header z-50)
WC Cancel clears spinner / re-enables CTA PARTIAL for store; Retry vs global WC disconnect is the hole

Security (no BLOCK)

Vector Result
Leftover fixed inset-0 covering Connect Fixed (portal + route dismiss)
Stuck connecting from previous tab Fixed (hydrate reset)
Late WC success after Cancel Store ghost-session fixed; Retry race above
WC URI / intent off allowlist Unchanged; still sound
Legal skip / clickjack Connect tap ≠ T&C accept
Simulated wallet in production UI DEV_MODE gated + new test; store connectSimulated() still ungated (pre-existing)
XSS / prompt injection Pass (React text; no dangerouslySetInnerHTML)

Nice-to-have (not merge blockers)

  • Single-flight connect() if connecting is already true (Retry already clears it first).
  • DEV_MODE guard on connectSimulated in the store.
  • E2E Cancel-abort and Verify-route backdrop; Escape-to-close dropdown.
  • Keep #137 open for Android 16 Chrome tap → modal → Open/Copy → terra1, plus desktop Keplr extension smoke.
RECOMMEND: FIX Follow-up to !159 for #137. Cancel-abort and header-safe backdrops are real improvements (connect epoch, `WalletMenuBackdrop` portaled at z-40 under header z-50, `useDismissOnNavigate`). No prompt injection, XSS, WC-allowlist regression, or Legal-gate skip. **Do not close #137** until Android 16 Chrome device QA. ## Must-fix **Stale `connectTerraWallet` success always calls `disconnectTerraWallet()`, which can kill Retry.** `packages/frontend/src/stores/wallet.ts` (epoch mismatch after `await connectTerraWallet`): ```ts if (epoch !== connectEpoch) { await disconnectTerraWallet() throw new ConnectionCancelledError() } ``` `connectTerraWallet` already wrote into `connectedWallets` before returning. `TerraWalletModal.handleRetry` does `cancelConnection()` then `connect()` 100ms later. Cosmes `KeplrController.disconnect` then calls `this.wc.disconnect()` when the controller map is empty — that drops the **shared** WalletConnect client under in-flight Retry. Unit tests only cover Cancel **without** a second `connect()` (`wallet.test.ts`). **Required:** skip protocol disconnect when `get().connecting` is true (a newer connect owns the singleton). Still throw `ConnectionCancelledError` and do **not** `set({ connected: true })`. Add a unit test: connect A → `cancelConnection` → connect B → resolve A → `connected` still false, B still running, `disconnectTerraWallet` **not** called while B is connecting. ## Issue #137 | AC | This MR | |----|---------| | Android 16 Chrome first tap opens modal | NOT MET (honestly deferred; Playwright UA ≠ device) | | `connecting === false` on fresh visit | MET (`applyWalletHydrateReset`) | | Lunc Dash / Galaxy Open+Copy | PARTIAL (!159 pairing; device QA still open) | | Keplr not a dead “Not installed” row | N/A (!159) | | Desktop extension unchanged | MET | | Legal gate does not swallow Connect | MET (TermsGate is transfer-only; header z-50) | | WC Cancel clears spinner / re-enables CTA | PARTIAL for store; Retry vs global WC disconnect is the hole | ## Security (no BLOCK) | Vector | Result | |--------|--------| | Leftover `fixed inset-0` covering Connect | Fixed (portal + route dismiss) | | Stuck `connecting` from previous tab | Fixed (hydrate reset) | | Late WC success after Cancel | Store ghost-session fixed; Retry race above | | WC URI / intent off allowlist | Unchanged; still sound | | Legal skip / clickjack | Connect tap ≠ T&C accept | | Simulated wallet in production | UI `DEV_MODE` gated + new test; store `connectSimulated()` still ungated (pre-existing) | | XSS / prompt injection | Pass (React text; no `dangerouslySetInnerHTML`) | ## Nice-to-have (not merge blockers) - Single-flight `connect()` if `connecting` is already true (Retry already clears it first). - `DEV_MODE` guard on `connectSimulated` in the store. - E2E Cancel-abort and Verify-route backdrop; Escape-to-close dropdown. - Keep #137 open for Android 16 Chrome tap → modal → Open/Copy → `terra1`, plus desktop Keplr extension smoke.
PlasticDigits commented 2026-09-01 09:04:04 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 4761fa5a - fix(frontend): skip WalletConnect disconnect while Retry is connecting (GL-137)

Compare with previous version

added 1 commit <ul><li>4761fa5a - fix(frontend): skip WalletConnect disconnect while Retry is connecting (GL-137)</li></ul> [Compare with previous version](/PlasticDigits/cl8y-bridge-monorepo/-/merge_requests/22/diffs?diff_id=1996247640&start_sha=f7e9ffd6ec86eba7b900afbab21c19860df1ea33)
PlasticDigits commented 2026-09-01 09:04:22 +00:00 (Migrated from gitlab.com)

Review must-fix addressed (4761fa5)

Must-fix from the FIX review: a stale connectTerraWallet success always called disconnectTerraWallet(), which can kill Retry. Cosmes KeplrController.disconnect drops the shared WalletConnect client when the controller map is empty. TerraWalletModal.handleRetry does cancelConnection() then connect() 100ms later.

Change: epoch mismatch still throws ConnectionCancelledError and does not set({ connected: true }). Protocol disconnect runs only when connecting === false (shouldDisconnectGhostWalletConnect / disconnectGhostWalletConnectIfUnowned). If Retry already owns the singleton, skip disconnectTerraWallet.

Cancel-without-Retry still disconnects the ghost session (existing unit test unchanged).

Completed from the review

  • Skip protocol disconnect when a newer connect() is in flight
  • Still throw ConnectionCancelledError; do not set connected
  • Unit test: connect A → cancelConnection → connect B → resolve A → connected false, B still connecting, disconnectTerraWallet not called
  • Documented INV-FE-WC-MOBILE-1 Cancel row, docs/frontend.md, QA 3.12, skills/agent-frontend-terra-wallet-mobile.md

Not done (review nice-to-haves / out of scope)

  • Single-flight connect() if connecting is already true
  • DEV_MODE guard on store connectSimulated() (UI already gated; store remains ungated)
  • New E2E for Cancel-abort / Verify-route backdrop / Escape-to-close dropdown (existing History backdrop e2e still passes)
  • Android 16 Chrome device QA and desktop Keplr extension smoke — keep #137 open

Verification already run on this commit

  • npm run test:unit -- src/stores/wallet.test.ts — 8 passed (retested)
  • Related GL-137 unit files — 10 files, 49 passed
  • npx tsc --noEmit / ESLint on changed TS — clean (pre-commit also passed)
  • E2E_UI_ONLY=1 npx playwright test e2e/wallet-connect.spec.ts --workers=5 — 9 passed

Checklist to verify

Store / Retry race

  • Cancel during WalletConnect wait: spinner clears, header CTA re-enables, pairing sheet closes
  • After Cancel without Retry, a late WC approve must not leave connected / terra1 in the header (ghost session disconnected)
  • Modal Retry after Cancel: pairing continues; a late success from the cancelled attempt must not drop the in-flight WC session (QA 3.12)
  • ConnectionCancelledError is not shown as connectionError and is not remapped to “install the extension”

Already on this MR (regression)

  • Fresh visit: connecting === false; header is Cancel while connecting, not a disabled spinner (QA 3.11)
  • Connected Terra/EVM/Solana dropdown: fixed inset-0 backdrop gone after History / Verify; Connect still tappable (QA 3.15)
  • Simulated Terra Wallet hidden when DEV_MODE is false
  • Desktop Keplr extension path unchanged
  • Legal TermsGate still does not swallow Connect (transfers only)

Device (do not close #137 until these pass)

  • Android 16 Chrome: first tap opens TerraWalletModal
  • Lunc Dash / Galaxy Station Open + Copy → terra1 in header
  • Overlay stacking screenshot from a real Android 16 Chrome session if useful
## Review must-fix addressed (`4761fa5`) **Must-fix from the FIX review:** a stale `connectTerraWallet` success always called `disconnectTerraWallet()`, which can kill Retry. Cosmes `KeplrController.disconnect` drops the shared WalletConnect client when the controller map is empty. `TerraWalletModal.handleRetry` does `cancelConnection()` then `connect()` 100ms later. **Change:** epoch mismatch still throws `ConnectionCancelledError` and does **not** `set({ connected: true })`. Protocol disconnect runs only when `connecting === false` (`shouldDisconnectGhostWalletConnect` / `disconnectGhostWalletConnectIfUnowned`). If Retry already owns the singleton, skip `disconnectTerraWallet`. Cancel-without-Retry still disconnects the ghost session (existing unit test unchanged). ### Completed from the review - [x] Skip protocol disconnect when a newer `connect()` is in flight - [x] Still throw `ConnectionCancelledError`; do not set `connected` - [x] Unit test: connect A → `cancelConnection` → connect B → resolve A → `connected` false, B still `connecting`, `disconnectTerraWallet` **not** called - [x] Documented **INV-FE-WC-MOBILE-1** Cancel row, `docs/frontend.md`, QA 3.12, `skills/agent-frontend-terra-wallet-mobile.md` ### Not done (review nice-to-haves / out of scope) - Single-flight `connect()` if `connecting` is already true - `DEV_MODE` guard on store `connectSimulated()` (UI already gated; store remains ungated) - New E2E for Cancel-abort / Verify-route backdrop / Escape-to-close dropdown (existing History backdrop e2e still passes) - Android 16 Chrome device QA and desktop Keplr extension smoke — **keep #137 open** ### Verification already run on this commit - `npm run test:unit -- src/stores/wallet.test.ts` — 8 passed (retested) - Related GL-137 unit files — 10 files, 49 passed - `npx tsc --noEmit` / ESLint on changed TS — clean (pre-commit also passed) - `E2E_UI_ONLY=1 npx playwright test e2e/wallet-connect.spec.ts --workers=5` — 9 passed --- ## Checklist to verify **Store / Retry race** - [ ] Cancel during WalletConnect wait: spinner clears, header CTA re-enables, pairing sheet closes - [ ] After Cancel **without** Retry, a late WC approve must **not** leave `connected` / `terra1` in the header (ghost session disconnected) - [ ] Modal **Retry** after Cancel: pairing continues; a late success from the *cancelled* attempt must not drop the in-flight WC session (QA 3.12) - [ ] `ConnectionCancelledError` is not shown as `connectionError` and is not remapped to “install the extension” **Already on this MR (regression)** - [ ] Fresh visit: `connecting === false`; header is Cancel while connecting, not a disabled spinner (QA 3.11) - [ ] Connected Terra/EVM/Solana dropdown: `fixed inset-0` backdrop gone after History / Verify; Connect still tappable (QA 3.15) - [ ] Simulated Terra Wallet hidden when `DEV_MODE` is false - [ ] Desktop Keplr extension path unchanged - [ ] Legal TermsGate still does not swallow Connect (transfers only) **Device (do not close #137 until these pass)** - [ ] Android 16 Chrome: first tap opens TerraWalletModal - [ ] Lunc Dash / Galaxy Station Open + Copy → `terra1` in header - [ ] Overlay stacking screenshot from a real Android 16 Chrome session if useful
PlasticDigits (Migrated from gitlab.com) merged commit 6e7a510c90 into main 2026-09-02 01:38:24 +00:00
PlasticDigits commented 2026-09-02 01:38:25 +00:00 (Migrated from gitlab.com)

mentioned in commit 6e7a510c90

mentioned in commit 6e7a510c90e3cd88fe43c004a431a6d0460007d9
PlasticDigits commented 2026-09-02 01:39:18 +00:00 (Migrated from gitlab.com)

mentioned in issue #139

mentioned in issue #139
Sign in to join this conversation.
No description provided.