feat(pair): add reason to the /pair choice screen view event by dschom · Pull Request #21010 · mozilla/fxa

dschom · 2026-08-12T01:10:24Z

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 pull request

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.

Issue that this pull request solves

Closes: FXA-14133

Checklist

Put an x in the boxes that apply

My commit is GPG signed.
If applicable, I have modified or added tests which pass locally.
I have added necessary documentation (if appropriate).
I have verified that my changes render correctly in RTL (if appropriate).
I have manually reviewed all AI generated code.

How to review (Optional)

Key files/areas to focus on:
Suggested review order:
Risky or complex parts:

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.

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 · 2026-08-12T17:35:15Z

        if (isSyncDesktopV3Integration(integration)) {
          const { to } = getSyncNavigate(location.search, {
            showSignupConfirmedSync: true,
+            origin: 'signup',


I think this is acting like a sinup action... but let's double check...

Looks like it is, I see the isSignup check above it that we pass in when it's a signup code.

LZoog · 2026-08-12T17:40:18Z


    expect(hardNavigateSpy).toHaveBeenCalledWith(
-      '/pair?showSuccessMessage=true'
+      '/pair?showSuccessMessage=true&pairReason=password_login'


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?

That makes sense. Initially Claude also included content server here, but I rolled those back and this fell through the cracks.

Ah... I see what's happening here. There's a code a path where can hard navigate. Are you suggesting that path should be removed entirely? If not, then this needs to change, cause we will be passing along the pairReason query parameter by default. The hard navigate is based on the to value returned by getSyncNavigate, which with changes will return password_login.

@dschom it looks like we never check the config there. We should have been checking, "if react pair is on from the config then normal navigate". I think it's fine to just remove at this point.

LZoog

@dschom This is a big diff for something I would've expected we could mostly get at least? looking at flow ID + other Glean events, but I can see why we'd want it.

I tested the password sign up and password sign in flows and checked the Glean pings and they looked right. I'll go ahead and r+ not to block you 👍

LZoog · 2026-08-13T15:39:26Z


+      // The dispatcher case arm is the only thing deciding what reaches Glean
+      // for this event, so both the populated and empty reason are pinned here
+      // (FXA-14133).


Not sure this comment is adding much, at least remove ticket reference?

LZoog · 2026-08-13T16:06:40Z

            handleFxaOAuthLogin: true,
            showSignupConfirmedSync: true,
            origin: 'post-verify-set-password',
+            // Sync needs keys, so every passwordless OTP/passkey sign-in lands


nit: non-PRF passkey sign-in

Though how you have it is true today

LZoog · 2026-08-14T17:36:38Z


    expect(hardNavigateSpy).toHaveBeenCalledWith(
-      '/pair?showSuccessMessage=true'
+      '/pair?showSuccessMessage=true&pairReason=password_login'


@dschom it looks like we never check the config there. We should have been checking, "if react pair is on from the config then normal navigate". I think it's fine to just remove at this point.

LZoog · 2026-08-14T18:01:01Z

+      // (FXA-14133).
+      it('submits a ping with the cad_firefox_choice_view event name and a reason', async () => {
+        GleanMetrics.cadFireFox.choiceView({
+          event: { reason: 'otp_login' },


This is just a nit but otp_login really made me think passwordless OTP login at first. otp_confirmation or some other name would make it more clear but I know these were set by Product so just a suggestion.

LZoog · 2026-08-14T18:15:27Z

        if (isSyncDesktopV3Integration(integration)) {
          const { to } = getSyncNavigate(location.search, {
            showSignupConfirmedSync: true,
+            origin: 'signup',


Looks like it is, I see the isSignup check above it that we pass in when it's a signup code.

LZoog · 2026-08-14T18:16:03Z

      );
    });

+    // This is the only producer of isPasskeySession, and therefore the only


Remove comment? This could be stale later

LZoog · 2026-08-14T18:44:16Z

+  'password_login',
+  'password_reg',
+  'otp_login',
+  'passkey_login',


I guess if a user accesses this directly from the Firefox browser menu, Product can tell by checking the entrypoint on the param and check that reason is blank. Going to firefox.com/pair directly (like we tell people to do to pair) and clicking on "Connect another device" from the browser both record a blank reason. Expected I think but just noting.

dschom requested a review from a team as a code owner August 12, 2026 01:10

dschom force-pushed the FXA-14133 branch from a257bb1 to 162785e Compare August 12, 2026 01:24

dschom marked this pull request as draft August 12, 2026 01:33

dschom force-pushed the FXA-14133 branch from 162785e to 606b503 Compare August 12, 2026 01:36

dschom force-pushed the FXA-14133 branch from 606b503 to 5e61ecd Compare August 12, 2026 15:29

dschom marked this pull request as ready for review August 12, 2026 15:30

dschom commented Aug 12, 2026

View reviewed changes

LZoog reviewed Aug 12, 2026

View reviewed changes

LZoog approved these changes Aug 14, 2026

View reviewed changes

Uh oh!

Conversation

dschom commented Aug 12, 2026

Because

This pull request

Issue that this pull request solves

Checklist

How to review (Optional)

Screenshots (Optional)

Other information (Optional)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dschom Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LZoog left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dschom Aug 12, 2026 •

edited

Loading