fix(e2e): complete new-user login past the 2FA-settings reminder - #1242
Open
aivong-openhands wants to merge 2 commits into
Open
fix(e2e): complete new-user login past the 2FA-settings reminder#1242aivong-openhands wants to merge 2 commits into
aivong-openhands wants to merge 2 commits into
Conversation
…t page The new-user setup project (setup/setup-new-user.ts) has been failing on unstable and beta since ~Sep 11 with a 60s page.waitForURL timeout in completeLoginAndOnboard, while the returning user authenticates fine. Root cause is in handleOAuthAuthorization. It submits GitHub's authorize form once and, if the form is not present in the DOM at that instant, assumes GitHub "already redirected" back to the app and returns. For a new user whose GitHub account has no standing OAuth grant, GitHub renders the first-time consent page and the authorize form can appear a beat after the URL settles. The single-shot submit missed it, the helper returned as a false "bypass", the browser stayed stranded on github.com, and the downstream wait timed out with a misleading stack. Make the grant robust: - Retry submitting the authorize form until the grant actually completes, defined as the page leaving the authorize URL (not merely "form submitted"). A missing form is a retryable state, not proof of success. - Broaden the form selector to also match an absolute action or any form carrying the authorize control, covering first-time-grant page variants. - Fail loudly with context if the grant never completes, instead of silently returning and surfacing as a 60s timeout elsewhere. - Export the helper and add offline unit tests (route-intercepted GitHub flow) covering the happy path, the late-rendering form (retry), the already-authorized path, and the stuck-grant regression. Note: this hardens the harness against the first-time-consent variant. The new-user GitHub account's OAuth grant going missing is external to the e2e code (keycloak-cleanup only deletes the Keycloak user, never the GitHub grant); confirm/reset it under that account's Authorized OAuth Apps. Co-authored-by: openhands <openhands@all-hands.dev>
…-user login Live-testing the new-user setup against unstable revealed the actual root cause of the failing `authenticate new user` step. Between 2FA entry and the OAuth grant, GitHub now interrupts the flow with a one-time "Verify your two-factor authentication (2FA) settings" reminder (served at /sessions/two-factor, offering "Verify 2FA now" or "skip 2FA verification — we'll remind you again tomorrow"). The harness never dismissed it, so the authorize form never rendered — which is exactly why the old code logged "form not found, already redirected" and then timed out downstream. Navigation order observed in the Playwright trace: /login/oauth/authorize -> /login -> /sessions/two-factor/app (TOTP) -> /sessions/two-factor (reminder) -> /login/oauth/authorize Fix: - Add dismissTwoFactorReminder(): clicks "skip 2FA verification" when the reminder is shown (skip resumes the interrupted flow and, unlike "Verify 2FA now", needs no extra TOTP entry, so it is safe to repeat every run). - Call it after 2FA in authenticateWithGitHub, and again inside the OAuth authorize retry loop (the reminder can also appear at the authorize URL). - Add offline unit tests for the reminder helper (route-intercepted fixture). - Run the unit tests in CI: add a `npm run test:unit` step to the E2E Static Checks workflow so the new-user auth helpers are actually guarded (the workflow previously only linted and listed tests). Verified end-to-end against https://app.unstable.staging.all-hands-testing.dev with the New User account: the run now dismisses the reminder, completes the OAuth grant on attempt 1, accepts TOS, and saves storage state — "1 passed". Co-authored-by: openhands <openhands@all-hands.dev>
aivong-openhands
changed the title
aivong-openhands
marked this pull request as ready for review
September 11, 2026 22:54
aivong-openhands
requested review from
dylan-openhands,
jlav and
mamoodi
as code owners
September 11, 2026 22:54
dylan-openhands
approved these changes
Sep 11, 2026
aivong-openhands
enabled auto-merge (squash)
September 12, 2026 02:32
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.
Why
The new-user e2e login (
setup/setup-new-user.ts→ "authenticate new user") has been failing while the returning-user path passes. Between 2FA and the OAuth grant, GitHub now shows a one-time "Verify your two-factor authentication (2FA) settings" reminder (/sessions/two-factor) for an account whose 2FA was recently configured. The harness never dismissed it, so the authorize form never rendered: the old handler read the missing form as "already redirected", returned, and the run sat on github.com until a 60s wait timed out.The login now clicks "skip 2FA verification" whenever that reminder appears — after 2FA and again inside the authorize loop, since it can also show at the authorize URL. Skipping resumes the interrupted flow and needs no extra TOTP, so it is safe to repeat on every run. The OAuth step now also retries the authorize-form submit until the page actually leaves the authorize URL, and throws with the current URL if it never does, instead of silently bypassing — a missing form is a retryable state, not success. The change touches only the e2e test harness and its CI check.
A
npm run test:unitstep is added to the E2E Static Checks workflow, which previously only linted and listed tests, so the new-user auth helpers now run in CI.Validation
setup:new-useragainst the staging app: it dismissed the 2FA reminder, completed the OAuth grant on the first attempt, accepted Terms of Service, and saved storage state (1 passed).npm run test:unitpassed (12), covering reminder dismissal and no-op plus the grant's happy-path, late-rendering-form retry, already-authorized, and stuck-grant cases.npm run lint(tsc, eslint, prettier) clean.This PR was drafted by an AI agent on behalf of the user.