fix(settings): skip redundant set-password account-status check#20719
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reduces first-paint latency (and stage smoke-test flakes) on the PostVerify Set Password page by skipping a redundant accountStatus round-trip when the page is reached via known forward flows that already imply a passwordless account.
Changes:
- Gate the blocking
accountStatuscheck based on presence of router state to allow immediate render for forward flows. - Simplify the
accountStatuseffect logic (flatthen/catch) and update the related inline comment. - Add tests to validate when the
accountStatuscheck is skipped vs executed, including fail-open behavior on errors.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/fxa-settings/src/pages/PostVerify/SetPassword/container.tsx | Skips blocking accountStatus check for state-routed forward flows to improve first paint; keeps redirect behavior for password-present cases. |
| packages/fxa-settings/src/pages/PostVerify/SetPassword/container.test.tsx | Adds unit tests covering the new gating behavior for accountStatus, including skip/run/error paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
vbudhram
approved these changes
Jun 10, 2026
vbudhram
left a comment
vbudhram
left a comment
Contributor
There was a problem hiding this comment.
@vpomerleau I didn't manually test this but code seems low risk and resonable.
vpomerleau
force-pushed
the
FXA-13937-skip-setpassword-check
branch
2 times, most recently
from
8dde7c5 to
876f8e0
Compare
…rward flows Because: - The post-verify set-password page blocked first paint on an accountStatus round-trip, intermittently exceeding the stage smoke test's 10s assertion and flaking passkeySetPassword.spec.ts. - The page only ever serves passwordless accounts, and the forward flows already know that, so the round-trip is redundant for them. This commit: - Trusts the account's stored password state instead of the blocking round-trip: renders for a passwordless account, redirects to /signin when one exists, and falls back to a one-time accountStatus check only when it is unknown (third-party sign-in, or a stale/direct arrival). - Records hasPassword in the flows that establish it as fact: passwordless OTP with or without TOTP (passkey already did). Third-party defers to the check. - Replaces the set-password history entry on completion so Back cannot return to it; flattens the check effect and drops the stale retry TODO and comment. - Adds container, handleNavigation, and SigninTotpCode tests. Closes #FXA-13937
vpomerleau
force-pushed
the
FXA-13937-skip-setpassword-check
branch
from
876f8e0 to
246f258
Compare
Contributor
Author
|
@vbudhram I made some changes based on the copilot comments - would you mind having another that this still looks good? |
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.
Because
accountStatusround-trip, which intermittently exceeded the stage smoke test's 10s assertion and flakedpasskeySetPassword.spec.ts.This pull request
accountStatuscheck on router state inpackages/fxa-settings/src/pages/PostVerify/SetPassword/container.tsx: when the page is reached via a forward flow (location.state.passwordCreationReasonis set), it paints immediately without the round-trip./signinis still wanted.then/catch, resolves the stale retry TODO, and corrects the inaccurate "third-party may have a password" comment.Issue that this pull request solves
Closes: FXA-13937
Checklist
Put an
xin the boxes that applyHow to review (Optional)
Screenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.
Other information (Optional)
Known pre-existing limitation (out of scope): on a stateless arrival where the
accountStatuscheck itself fails and the account already has a password, the page falls through to the form and/password/createrejects server-side with a "password already set" banner instead of redirecting to/signin. This requires a rare triple-coincidence and is a candidate for a follow-up.