Conversation
Because: - Users reach the /pair choice screen from several flows (password sign-in, registration, email OTP sign-in, passkey sign-in), and cad_firefox.choice_view carries no way to tell them apart. - Splitting that funnel by originating flow needs the attribution on the view event itself, rather than inferred by joining across other events. This commit: - Adds a `reason` extra key to cad_firefox.choice_view and regenerates the Glean bindings. The permitted values live in one place, fxa-shared/metrics/glean/pair-reasons.ts, which also exports the runtime guard both /pair implementations validate with. - Derives the reason in getSyncNavigate. `passwordCreationReason` is the primary signal: Sync always requires encryption keys, so every passwordless OTP or passkey sign-in is routed through /post_verify/set_password before it can reach /pair, making that page the only place that still knows how the session was established. The raw session flags (isPasskeySession, isPasswordlessOtpSignin) cover the flows that skip it. - Resolves the reason before the interstitial early returns so it rides along on /signup_confirmed_sync and /inline_recovery_key_setup, which reach /pair via hardNavigate('/pair', {}, true). Without this the reason was lost for every flow that stops at an interstitial, and password_reg was unreachable outside Send Tab. - Declares origin: 'signup' at the sign-up callers that set showSignupConfirmedSync, which otherwise fell through to password_login. - Reports no reason for third-party auth rather than folding it into password_login; it has no bucket of its own in FXA-14133. - Reads the value in React /pair from router state with a query-param fallback for the interstitial hand-off, validating both. The reason is deliberately kept out of the soft-nav URL so an account-level attribute is not written to history or server access logs on every pair visit. - Validates the Backbone query param against the shared allowlist before recording it, since that value arrives from the URL. - Adds handleNavigation-level tests asserting the reason that actually reaches /pair. The pass-through had no coverage, so the derivation could be wired to signals no Sync flow carries and the suite would still pass. - Parses app/scripts/lib/glean/*.ts with @typescript-eslint/parser in fxa-content-server (precedent: fxa-profile-server). That directory is TypeScript while the app is otherwise babel-eslint, so the dispatcher could not be linted at all; scoped to that directory to avoid surfacing pre-existing violations in unrelated .ts files.
dschom
marked this pull request as ready for review
August 12, 2026 15:30
dschom
commented
Aug 12, 2026
| if (isSyncDesktopV3Integration(integration)) { | ||
| const { to } = getSyncNavigate(location.search, { | ||
| showSignupConfirmedSync: true, | ||
| origin: 'signup', |
Contributor
Author
There was a problem hiding this comment.
I think this is acting like a sinup action... but let's double check...
LZoog
reviewed
Aug 12, 2026
|
|
||
| expect(hardNavigateSpy).toHaveBeenCalledWith( | ||
| '/pair?showSuccessMessage=true' | ||
| '/pair?showSuccessMessage=true&pairReason=password_login' |
Contributor
There was a problem hiding this comment.
The only reason you would need this would be for Backbone pairing. I don't see any changes in fxa-content-server (and don't think we need them) so, remove?
Contributor
Author
There was a problem hiding this comment.
That makes sense. Initially Claude also included content server here, but I rolled those back and this fell through the cracks.
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
This pull request
reasonextra key to cad_firefox.choice_view and regenerates the Glean bindings. The permitted values live in one place, fxa-shared/metrics/glean/pair-reasons.ts, which also exports the runtime guard both /pair implementations validate with.passwordCreationReasonis the primary signal: Sync always requires encryption keys, so every passwordless OTP or passkey sign-in is routed through /post_verify/set_password before it can reach /pair, making that page the only place that still knows how the session was established. The raw session flags (isPasskeySession, isPasswordlessOtpSignin) cover the flows that skip it.Issue that this pull request solves
Closes: FXA-14133
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)
Any other information that is important to this pull request.