test(e2e): reconnect after session expiry with data-loss verification - #385
Merged
davedumto merged 1 commit intoAug 30, 2026
Conversation
|
@onuibeblessing2019-hash Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
@onuibeblessing2019-hash is attempting to deploy a commit to the david's projects Team on Vercel. A member of the Team first needs to authorize it. |
Author
|
closes #321 |
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
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.
Closes #321
Important scope disclosure — read before reviewing
This repo has no client-side session-expiry detection today:
WalletSession(@vellar/types) carries noexpiresAtfield.apps/web/lib/wallet-context.tsxnever checks for a 401 and never auto-triggers a reconnect prompt.services/wallet-service/src/server.ts'sSESSION_TTL_MS/resolveSessionCapability) is exercised only by that service's own existing unit test (server.test.ts: "an EXPIRED session bearer is treated as ABSENT") — it is never surfaced to the UI.idea.md(referenced bywallet.spec.ts's own existing comment, "§15") does not exist anywhere in this repository as of writing — could not verify the scenario's exact original wording.apps/extension/lib/state.ts'sPairedWallet) is a pairing (address/network/RPC/origin) that never expires on its own; the closest thing, the device signer's own 7-day expiry (signer-expiration.ts), is an on-chain signing-authority bound enforced by the contract, not a UI flow. There is nothing there to write a meaningful "extension reconnect" test against, so I did not fabricate one — flagging this explicitly rather than silently skipping the extension half of the issue's ask.Given that, there is currently no UI behavior that distinguishes "your session expired" from a manual disconnect — both look identical to a user (no persisted session; "Sign in" is the only way back). This PR's new test therefore uses the same trigger the existing "create, fund, pay, reconnect" scenario already uses (manual disconnect, the only reconnect trigger the app actually implements) as the stand-in for expiry, and focuses its actual new value on the part of #321's acceptance criteria the existing test does NOT check: verifying no data loss across reconnect.
What's here
apps/web/e2e/wallet.spec.ts:enableVirtualAuthenticatorhelper (used by both the pre-existing test and the new one — was duplicated inline before).Why this couldn't be executed here
Per
playwright.config.ts's own comment, this suite requiresapi-gateway(:4000) andwallet-service(:4001) running with real relayer + Postgres config, against live Stellar testnet with real friendbot funding. None of that infrastructure is available in this environment. Verified as much as is possible without it:npx playwright test --list— both tests parse and are discovered correctly (2 tests in 1 file), confirming no syntax/structural errors.npx tsc --noEmitinapps/web— 20 errors, byte-identical before and after this change (confirmed via a stash/restore diff) — zero new errors introduced. All 20 are pre-existing and unrelated (aWalletSession.contractId/sessionIdmismatch indashboard/page.tsx, strict-null issues inanalytics.test.ts, and theworkerProcessingLagSeconds is not definedbug inpackages/service-kitalready flagged and fixed independently in feat: RPC timeout budgets for policy deploy + circuit breaker for verification-service #381/fix(lifecycle-service): invalidate cached account reads after merge #384).I was not able to run this against real testnet infrastructure — flagging clearly so a reviewer with that environment available can do a real run before relying on this test in CI.
Closes #321