Fix/202: e2e logout relogin onboarding#305
Merged
graycyrus merged 2 commits intotinyhumansai:mainfrom Apr 3, 2026
Merged
Conversation
…ers (tinyhumansai#202) - Add isOnboardingOverlayVisible, waitForOnboardingOverlayVisible, waitForOnboardingOverlayHidden to shared-flows for precise overlay lifecycle assertions across specs. - Add logoutViaSettings helper: navigates to Settings, clicks Log out, handles optional confirmation dialog, asserts logged-out state — consolidating logout logic that was duplicated per-spec. - Add waitForLoggedOutState helper returning the first welcome-screen marker found (Welcome / Sign in / Login / Get Started). - New spec logout-relogin-onboarding.spec.ts verifies: 1. Fresh login completes onboarding and reaches Home. 2. Logout clears persisted auth/onboarding state (token + all per-user onboarding maps reset to {}). 3. Re-login with a delayed /telegram/me response does NOT show the overlay prematurely (proves no stale userLoadTimedOut leak). 4. Once the fresh-session timeout elapses the overlay appears with clean Welcome + Skip markers and a /telegram/me call is made. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ests (tinyhumansai#202) - Add getDelayMs/sleep utilities to mock-api-core.mjs so individual endpoints can honour a configurable delay set via __admin/behavior. - Wire telegramMeDelayMs into the GET /telegram/me handler so the logout-relogin spec can simulate a slow profile fetch and assert that the onboarding overlay does not fire before the timeout threshold. - Reformat long if-conditions and json() call sites to stay within line length limits (no logic changes). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughNew E2E test helpers detect onboarding overlay visibility with polling waiter functions and implement logout-via-settings workflow with state verification. A test spec validates logout→relogin onboarding behavior by monitoring browser storage and network requests. Mock API gains optional delay support for Telegram profile fetch simulation. Changes
Sequence Diagram(s)sequenceDiagram
actor Test as E2E Test
participant App as App/Browser
participant Storage as localStorage
participant Settings as Settings UI
participant API as Mock API
participant Overlay as Onboarding Overlay
Test->>App: Boot app & login (mocked server)
App->>Storage: Store auth token + onboarding state
Test->>Settings: logoutViaSettings()
Settings->>App: Navigate to Settings
App->>Settings: Display Settings UI
Settings->>App: Click logout action
Settings->>Settings: Confirm logout dialog
Settings->>Storage: Clear persisted auth
Test->>Storage: Poll for auth cleared (token absent)
Storage-->>Test: Confirm auth cleared
Test->>App: Trigger re-login (deep link)
App->>API: POST /telegram/login-tokens/consume
API-->>App: Token consumed
App->>API: GET /telegram/me (with delay)
API-->>App: Profile data
App->>Storage: Store new auth token
Test->>Storage: Poll for new token & onboarding state
Storage-->>Test: Confirm token present, onboarding empty
Test->>Overlay: Check onboarding NOT visible (immediately)
Overlay-->>Test: Confirmed hidden
Test->>Overlay: waitForOnboardingOverlayVisible()
Overlay-->>Test: Overlay appears
Test->>App: Verify onboarding UI (Welcome, Skip)
App-->>Test: Assertion passed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/test/e2e/specs/logout-relogin-onboarding.spec.ts (2)
171-173: Double-parsing of already-decoded values.
getPersistedAuthSnapshot()already decodes the persisted values via its internaldecodefunction. CallingparsePersistedValue()again on lines 171-173 is redundant — if the values are already objects,parsePersistedValuewill return them unchanged, so this is harmless but unnecessary.♻️ Remove redundant parsePersistedValue calls
- expect(parsePersistedValue(secondSessionState.isOnboardedByUser)).toEqual({}); - expect(parsePersistedValue(secondSessionState.onboardingTasksByUser)).toEqual({}); - expect(parsePersistedValue(secondSessionState.hasIncompleteOnboardingByUser)).toEqual({}); + expect(secondSessionState.isOnboardedByUser).toEqual({}); + expect(secondSessionState.onboardingTasksByUser).toEqual({}); + expect(secondSessionState.hasIncompleteOnboardingByUser).toEqual({});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/test/e2e/specs/logout-relogin-onboarding.spec.ts` around lines 171 - 173, The three assertions redundantly call parsePersistedValue on values already decoded by getPersistedAuthSnapshot; remove the extra parsing and assert the decoded fields directly (e.g., check secondSessionState.isOnboardedByUser, secondSessionState.onboardingTasksByUser, and secondSessionState.hasIncompleteOnboardingByUser are equal to {}), keeping the same expect(...).toEqual({}) semantics and leaving getPersistedAuthSnapshot and parsePersistedValue used elsewhere unchanged.
38-46: Duplicate decode logic —parsePersistedValueis defined but the same logic is inlined ingetPersistedAuthSnapshot.The
decodearrow function insidegetPersistedAuthSnapshot(lines 55-61) duplicatesparsePersistedValue(lines 38-46). Consider removing the inlined version and reusingparsePersistedValue.However, note that
parsePersistedValueis defined outside thebrowser.execute()scope, so it cannot be called from within that callback. If you want to reuse it, the decode must happen after the execute returns.♻️ Optional: Refactor to avoid duplication
async function getPersistedAuthSnapshot() { - return browser.execute(() => { + const raw = await browser.execute(() => { const raw = window.localStorage.getItem('persist:auth'); if (!raw) return null; - - try { - const parsed = JSON.parse(raw); - const decode = value => { - if (typeof value !== 'string') return value; - try { - return JSON.parse(value); - } catch { - return value.replace(/^"|"$/g, ''); - } - }; - - return { - token: decode(parsed.token), - isOnboardedByUser: decode(parsed.isOnboardedByUser), - onboardingTasksByUser: decode(parsed.onboardingTasksByUser), - hasIncompleteOnboardingByUser: decode(parsed.hasIncompleteOnboardingByUser), - isAnalyticsEnabledByUser: decode(parsed.isAnalyticsEnabledByUser), - }; - } catch { - return null; - } + return raw; }); + if (!raw) return null; + + try { + const parsed = JSON.parse(raw); + return { + token: parsePersistedValue(parsed.token), + isOnboardedByUser: parsePersistedValue(parsed.isOnboardedByUser), + onboardingTasksByUser: parsePersistedValue(parsed.onboardingTasksByUser), + hasIncompleteOnboardingByUser: parsePersistedValue(parsed.hasIncompleteOnboardingByUser), + isAnalyticsEnabledByUser: parsePersistedValue(parsed.isAnalyticsEnabledByUser), + }; + } catch { + return null; + } }Also applies to: 55-61
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/test/e2e/specs/logout-relogin-onboarding.spec.ts` around lines 38 - 46, parsePersistedValue duplicates the inline decode inside getPersistedAuthSnapshot (the decode arrow in the browser.execute callback); remove the inlined decode from the execute callback and instead return the raw string values from browser.execute, then after the execute returns map over those results and call parsePersistedValue to decode them (since parsePersistedValue is defined outside the browser.execute scope and cannot be called inside the browser context); update getPersistedAuthSnapshot to perform decoding post-execute using parsePersistedValue and delete the duplicated decode logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/test/e2e/specs/logout-relogin-onboarding.spec.ts`:
- Around line 171-173: The three assertions redundantly call parsePersistedValue
on values already decoded by getPersistedAuthSnapshot; remove the extra parsing
and assert the decoded fields directly (e.g., check
secondSessionState.isOnboardedByUser, secondSessionState.onboardingTasksByUser,
and secondSessionState.hasIncompleteOnboardingByUser are equal to {}), keeping
the same expect(...).toEqual({}) semantics and leaving getPersistedAuthSnapshot
and parsePersistedValue used elsewhere unchanged.
- Around line 38-46: parsePersistedValue duplicates the inline decode inside
getPersistedAuthSnapshot (the decode arrow in the browser.execute callback);
remove the inlined decode from the execute callback and instead return the raw
string values from browser.execute, then after the execute returns map over
those results and call parsePersistedValue to decode them (since
parsePersistedValue is defined outside the browser.execute scope and cannot be
called inside the browser context); update getPersistedAuthSnapshot to perform
decoding post-execute using parsePersistedValue and delete the duplicated decode
logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9d8365ae-81b3-4ed6-89de-123187cfaa93
📒 Files selected for processing (3)
app/test/e2e/helpers/shared-flows.tsapp/test/e2e/specs/logout-relogin-onboarding.spec.tsscripts/mock-api-core.mjs
graycyrus
approved these changes
Apr 3, 2026
2 tasks
This was referenced Apr 7, 2026
Closed
4 tasks
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
2 participants
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.
To be merged after : #303
Summary
logout-relogin-onboarding.spec.ts) covering the full logout → re-login onboarding overlay lifecycle, including a stale-state regression assertion.logoutViaSettings,waitForLoggedOutState, and three overlay lifecycle helpers toshared-flows.tsso logout and overlay assertions are reusable across specs.telegramMeDelayMsbehavior to the mock server so tests can simulate a slow profile fetch and assert the overlay does not appear prematurely.Problem
No E2E test covered what happens after a user logs out and logs back in. Persistent flags like
userLoadTimedOutwritten by a previous session can survive across the logout boundary. If those flags are still set when the new session boots, the onboarding overlay either fires too early (before the user profile loads) or not at all — both are regressions that went undetected.The logout flow was also duplicated inline in
auth-access-control.spec.tswith no shared abstraction for other specs to use.Solution
New helpers in
shared-flows.ts:isOnboardingOverlayVisible()onboardingOverlayLikelyVisible— for point-in-time assertionswaitForOnboardingOverlayVisible(timeout)waitForOnboardingOverlayHidden(timeout)waitForLoggedOutState(timeout)logoutViaSettings(logPrefix)New spec
logout-relogin-onboarding.spec.ts— single test case with four checkpoints:logoutViaSettingsclears the session;persist:authin localStorage is verified to have an empty token and empty per-user onboarding maps — no stale state.telegramMeDelayMs=4500delays the profile response. Immediately after auth bootstrap the overlay must not be visible (proves no staleuserLoadTimedOutfrom the previous session fired early).Welcome+Skipmarkers, and a/telegram/mecall is recorded.Mock server changes (
mock-api-core.mjs):getDelayMs(key)/sleep(ms)utilities for configurable per-endpoint delays.telegramMeDelayMswired into theGET /telegram/mehandler.if-conditions andjson()call sites reformatted to stay within line length (no logic changes).Submission Checklist
Impact
logoutViaSettingsconsolidates logout logic that was previously duplicated inauth-access-control.spec.ts— future specs can use it directly.telegramMeDelayMsis available to any spec that needs to simulate slow profile fetches.Merge order note
Related
Summary by CodeRabbit