Web Wallet E2E (MetaMask/Snap): general improvements

[tuliomir]

Web Wallet E2E (MetaMask/Snap): follow-ups after PR #169

Tracking issue for improvements identified while reviewing #169, which added the real-MetaMask-Flask Playwright E2E suite under packages/web-wallet/tests/e2e/. None of these blocked that PR — it landed intentionally as on-demand, headed, non-gating infrastructure. This issue is the durable home for the next iteration, written against a main where #169 has already merged.

Ordered by value. Each item is independent; a future PR can pick off any subset.

---

1. Clean checkout can't run the suite from the docs alone — missing dependency build ⛔️ highest value

Following the "How to run" docs literally, from a clean tree, all four journeys fail at connect. Root cause: nothing builds the workspace dependency @hathor/hathor-rpc-handler. Its package.json points main at dist/index.js, but the repo root package.json has no scripts and there is no repo-wide build, so the snap's mm-snap watch dev build dies with:

Module not found: Can't resolve '@hathor/hathor-rpc-handler'

The snap bundle is then broken and every journey fails at warmSnap → connect. The missing step:

yarn workspace @hathor/hathor-rpc-handler build   # produces dist/index.js

This matters beyond convenience: the E2E build consumes @hathor/hathor-rpc-handler through its built dist, so anyone running the suite without this step is silently exercising stale handler code — including any future PR that changes packages/hathor-rpc-handler/src/** and expects the snap to pick it up.

Suggested fix: either add the build step to the "How to run" section, or (better) add a root build/prepare script (or a pretest/pree2e hook) so yarn e2e is self-sufficient from a clean checkout. A CI job — even a non-gating one — would have caught this immediately.

---

2. Determinism & the merge-bar contract for a non-gating suite

Empirically confirmed: even with the timeout hardening already in the suite, a single clean run on a slow headless Linux box was 2 passed / 3 failed (~12 min). The non-determinism is real — the same testnet network-switch passed in import (39s) but hit the 180s test timeout as the first step of both feature-example and token-lifecycle.

This is fine for on-demand infra, but the suite's status as explicitly non-gating currently lives mostly in the PR description, which disappears from view once merged. Future contributors will find a red suite and not know whether that's expected.

Suggested direction:

• State the non-gating contract where a contributor actually looks: a README/header comment in tests/e2e/ and/or the docs, not just the PR body.
• Track a concrete de-flake plan: the dominant failure mode is cold-start of the Snap's first heavy op (connect+install, testnet switch). Candidate mitigations — a readiness probe on the Snap/wallet before starting the timed connect, or a warm-up project that runs once per worker.
• Decide the trigger for investing in headless + CI (currently deferred). Until then, a nightly non-blocking CI run purely for signal would surface rot (e.g. item 1) without gating anyone.

---

3. A hard deadline escapes the centralized-timeout system

driver/timeouts.ts + E2E_TIMEOUT_SCALE centralize the driver's deadlines, and SNAP_TIMEOUT_MS covers the dApp's snap-RPC cap — good. But at least one load-bearing wait sits outside both: onboarding failed on a fixed 60s expectConnected() at packages/web-wallet/tests/e2e/helpers/webWallet.ts:119 (getByText(/assets summary/i) never visible), even with SNAP_TIMEOUT_MS=60000 applied.

Suggested fix: route that wait (and audit the page objects for siblings) through TIMEOUTS/E2E_TIMEOUT_SCALE so scaling for a slow machine actually scales every deadline, not just the driver's. Also worth a one-paragraph doc note distinguishing the two knobs — SNAP_TIMEOUT_MS (dApp side, the 10s RPC cap) vs E2E_TIMEOUT_SCALE (driver side) — and when to reach for each.

---

4. Fail-fast on recovery-word read may be too eager for the slow-render target

26b8573 added a throw when any recovery word reads empty (around packages/web-wallet/tests/e2e/driver/MetaMaskDriver.ts:164). Failing fast on a genuinely missing word beats an opaque downstream quiz timeout — the right instinct. But it's a one-shot read of the chip elements with no wait/retry for population, and slow renders are precisely the environment this suite targets, so a chip that simply hasn't populated yet can turn a recoverable transient into a hard failure.

Suggested fix: poll/retry the chip read until all 12/24 words are non-empty (bounded by a TIMEOUTS deadline) before declaring failure. Then the throw fires only on a real missing word.

---

5. SNAP_TIMEOUT_MS is production dApp behavior — pin it with a unit test

SNAP_TIMEOUT_MS (wired in packages/web-wallet/webpack.config.cjs:118 via DefinePlugin, default empty → 10s unchanged) makes the dApp's hard snap-RPC timeout overridable. It's the one change in this test-focused body that reaches production behavior. It defaults to a no-op, but that no-op isn't currently protected.

Suggested fix: a small unit test asserting the empty/unset default resolves to the original 10s, so a future refactor can't silently change the shipped timeout. Cheap insurance for a prod-facing knob.

---

6. Downloaded MetaMask Flask build — integrity & pinning

packages/web-wallet/tests/e2e/driver/flask.ts resolves and downloads a MetaMask Flask build from GitHub (E2E_FLASK_VERSION, with E2E_METAMASK_PATH as a local escape hatch). Running a downloaded browser extension is a real trust surface even in dev/CI.

Suggested direction: verify the download against a checksum/hash and prefer an immutable artifact over a mutable tag, so a compromised or swapped release can't silently execute in a contributor's browser. Not urgent for on-demand local use, but worth a conscious decision before this ever runs in CI.

---

7. Committed testnet seed + permanent testnet side-effects — hygiene

packages/web-wallet/tests/e2e/wallets.config.json commits a funded testnet SRP, and runs create tokens / send txs that persist on testnet (unique names per run). All reasonable for a throwaway testnet wallet, but worth making the intent explicit and durable:

• A comment/doc line stating the seed is testnet-only and its funds are disposable, so nobody mistakes it for a real secret or reuses it.
• Acknowledge the slow accumulation of test data / gradual fund drain on testnet, and note the top-up path when the funded wallet runs dry (right now a dry wallet just fails the funded journeys).

---

8. Version-pinned selectors — surface the upgrade path

packages/web-wallet/tests/e2e/driver/selectors.ts is pinned to MetaMask Flask 13.31.0. When MetaMask bumps, selectors rot and failures look mysterious.

Suggested fix: a short "upgrading the pinned MetaMask version" checklist in the docs (which selectors/testids to re-verify), and make the pin loud enough that a silent MetaMask auto-update produces an obvious "unsupported version" signal rather than a baffling mid-journey failure.

---

Appendix — empirical run (clean cache, slow headless Linux, DISPLAY=:0)

• Node 24 (docs assume Nix Node 22); yarn install clean, only pre-existing peer-dep warnings.
• Run following docs verbatim: 4/4 failed at connect (item 1).
• Run after yarn workspace @hathor/hathor-rpc-handler build: 2 passed / 3 failed, ~12 min. import ✅; onboarding, feature-example, token-lifecycle ❌ on cold-start/approval timeouts.
• Representative failures: expect(locator).toBeVisible() 60s timeout on the connected-home marker (webWallet.ts:119); Target page/context/browser has been closed at MetaMaskDriver.ts:611 readButtons during switchNetwork/confirmDialog.
• DISPLAY=:0 was sufficient; xvfb not required.