refactor(featureFlags): centralize ?ff= reading with the other query-state readers

The session override module read window.location.search through its own
URLSearchParams on every flag read, which is the one query parameter in the
app not read where all the others are.

Capture now happens in the router guard next to captureOAuthRequestId, taking
the LocationQuery the guard already has, and the new readCurrentLocationQuery
covers the reads that land before the router's first navigation - api.init(),
the auth store's sign-in path, and the router's own guards.

Fixes FE-1551

Author: Glary-Bot

src/platform/navigation/currentLocationQuery.ts

@@ -0,0 +1,15 @@
+import { parseQuery } from 'vue-router'
+import type { LocationQuery } from 'vue-router'
+
+/**
+ * The query of the URL as it stands right now, in the shape a router guard
+ * receives, for query-state readers that also have to answer before the
+ * router's first navigation.
+ *
+ * Going through the router's own parser rather than `URLSearchParams` is what
+ * makes the two paths agree: a bare `?flag` arrives as null from both, and a
+ * repeated parameter as an array from both.
+ */
+export function readCurrentLocationQuery(): LocationQuery {
+  return parseQuery(window.location.search)
+}

src/router.ts

@@ -20,6 +20,7 @@ import { installDesktopLoginRedemption } from '@/platform/cloud/onboarding/deskt
 import { installPreservedQueryTracker } from '@/platform/navigation/preservedQueryTracker'
 import { PRESERVED_QUERY_NAMESPACES } from '@/platform/navigation/preservedQueryNamespaces'
 import { preserveLoggedOutShareAuthAttribution } from '@/platform/workflow/sharing/utils/shareAuthAttribution'
+import { captureFeatureFlagOverrides } from '@/utils/sessionFeatureFlagOverride'
 
 const cloudOnboardingRoutes = isCloud
   ? (await import('./platform/cloud/onboarding/onboardingCloudRoutes'))
@@ -133,6 +134,7 @@ installPreservedQueryTracker(router, [
 ])
 
 router.beforeEach((to, _from, next) => {
+  captureFeatureFlagOverrides(to.query)
   captureOAuthRequestId(to.query)
   next()
 })

src/scripts/api.sessionOverride.test.ts

@@ -1,6 +1,8 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 
+import { readCurrentLocationQuery } from '@/platform/navigation/currentLocationQuery'
 import { api } from '@/scripts/api'
+import { captureFeatureFlagOverrides } from '@/utils/sessionFeatureFlagOverride'
 
 const mockDistribution = vi.hoisted(() => ({
   isCloud: true,
@@ -15,6 +17,11 @@ vi.mock('vuefire', () => ({
   useCurrentUser: vi.fn(() => mockCurrentUser)
 }))
 
+function visit(search: string) {
+  window.history.replaceState({}, '', search)
+  captureFeatureFlagOverrides(readCurrentLocationQuery())
+}
+
 /**
  * Every call here happens at plain module scope — no component, no `setup()`,
  * no active Vue instance — which is how `api.getServerFeature` is reached from
@@ -44,29 +51,29 @@ describe('api.getServerFeature session override outside component setup', () =>
 
   it('applies a numeric override to a flag that never routes through resolveFlag', () => {
     api.serverFeatureFlags.value = { max_upload_size: 100 }
-    window.history.replaceState({}, '', '/?ff=max_upload_size:209715200')
+    visit('/?ff=max_upload_size:209715200')
 
     expect(api.getServerFeature('max_upload_size')).toBe(209715200)
   })
 
   it('does not throw when no Vue instance is active', () => {
-    window.history.replaceState({}, '', '/?ff=some_flag:enforce')
+    visit('/?ff=some_flag:enforce')
 
     expect(() => api.getServerFeature('some_flag')).not.toThrow()
     expect(api.getServerFeature('some_flag')).toBe('enforce')
   })
 
   it('beats the dev localStorage override', () => {
     localStorage.setItem('ff:some_flag', '"from_local_storage"')
-    window.history.replaceState({}, '', '/?ff=some_flag:from_url')
+    visit('/?ff=some_flag:from_url')
 
     expect(api.getServerFeature('some_flag')).toBe('from_url')
   })
 
   it('withholds the override from a non-employee', () => {
     mockCurrentUser.value = { email: 'someone@gmail.com', emailVerified: true }
     api.serverFeatureFlags.value = { max_upload_size: 100 }
-    window.history.replaceState({}, '', '/?ff=max_upload_size:209715200')
+    visit('/?ff=max_upload_size:209715200')
 
     expect(api.getServerFeature('max_upload_size')).toBe(100)
   })
@@ -80,14 +87,14 @@ describe('api.getServerFeature session override outside component setup', () =>
 
   it('reports the override through serverSupportsFeature too', () => {
     api.serverFeatureFlags.value = { some_flag: false }
-    window.history.replaceState({}, '', '/?ff=some_flag')
+    visit('/?ff=some_flag')
 
     expect(api.serverSupportsFeature('some_flag')).toBe(true)
   })
 
   it('turns a supported feature off through an explicit false override', () => {
     api.serverFeatureFlags.value = { some_flag: true }
-    window.history.replaceState({}, '', '/?ff=some_flag:false')
+    visit('/?ff=some_flag:false')
 
     expect(api.serverSupportsFeature('some_flag')).toBe(false)
     expect(api.getServerFeature('some_flag')).toBe(false)

src/utils/sessionFeatureFlagOverride.test.ts

@@ -1,6 +1,10 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 
-import { getSessionOverride } from '@/utils/sessionFeatureFlagOverride'
+import { readCurrentLocationQuery } from '@/platform/navigation/currentLocationQuery'
+import {
+  captureFeatureFlagOverrides,
+  getSessionOverride
+} from '@/utils/sessionFeatureFlagOverride'
 
 const mockDistribution = vi.hoisted(() => ({
   isCloud: true,
@@ -20,8 +24,10 @@ vi.mock('vuefire', () => ({
 const COMFY_EMPLOYEE = { email: 'dev@comfy.org', emailVerified: true }
 const STORAGE_KEY = 'Comfy.FeatureFlagOverride'
 
+/** Stands in for a router navigation: URL change plus the guard's capture. */
 function visit(search: string) {
   window.history.replaceState({}, '', search)
+  captureFeatureFlagOverrides(readCurrentLocationQuery())
 }
 
 describe('getSessionOverride', () => {
@@ -89,6 +95,22 @@ describe('getSessionOverride', () => {
     expect(getSessionOverride('onboarding_tour_enabled')).toBeUndefined()
   })
 
+  it('clears the session on a bare ?ff with no equals sign', () => {
+    visit('/?ff=onboarding_tour_enabled')
+    expect(getSessionOverride('onboarding_tour_enabled')).toBe(true)
+
+    visit('/?ff')
+    expect(getSessionOverride('onboarding_tour_enabled')).toBeUndefined()
+  })
+
+  it('accumulates overrides across separate ?ff= visits', () => {
+    visit('/?ff=onboarding_tour_enabled')
+    visit('/?ff=signup_turnstile:enforce')
+
+    expect(getSessionOverride('onboarding_tour_enabled')).toBe(true)
+    expect(getSessionOverride('signup_turnstile')).toBe('enforce')
+  })
+
   it('overrides any flag, with no opt-in registry to join', () => {
     visit('/?ff=unified_cloud_auth&ff=some_flag_invented_tomorrow:42')
 
@@ -214,4 +236,40 @@ describe('getSessionOverride', () => {
       expect(sessionStorage.getItem(STORAGE_KEY)).toBeNull()
     })
   })
+
+  describe('capture from a router query', () => {
+    it('takes the query the guard hands it, without reading the URL', () => {
+      captureFeatureFlagOverrides({ ff: 'signup_turnstile:enforce' })
+
+      expect(window.location.search).toBe('')
+      expect(getSessionOverride('signup_turnstile')).toBe('enforce')
+    })
+
+    it('applies every value of a repeated parameter', () => {
+      captureFeatureFlagOverrides({
+        ff: ['workflow_sharing_enabled', 'signup_turnstile:shadow']
+      })
+
+      expect(getSessionOverride('workflow_sharing_enabled')).toBe(true)
+      expect(getSessionOverride('signup_turnstile')).toBe('shadow')
+    })
+
+    it('leaves an established session alone on a query without ff', () => {
+      visit('/?ff=onboarding_tour_enabled')
+
+      captureFeatureFlagOverrides({ workflowId: 'abc' })
+
+      expect(getSessionOverride('onboarding_tour_enabled')).toBe(true)
+    })
+
+    it('captures the URL for flags read before the router navigates', async () => {
+      window.history.replaceState({}, '', '/?ff=onboarding_tour_enabled')
+      vi.resetModules()
+
+      const { getSessionOverride: readWithoutRouter } =
+        await import('@/utils/sessionFeatureFlagOverride')
+
+      expect(readWithoutRouter('onboarding_tour_enabled')).toBe(true)
+    })
+  })
 })

src/utils/sessionFeatureFlagOverride.ts

@@ -1,6 +1,9 @@
+import { stringifyQuery } from 'vue-router'
+import type { LocationQuery } from 'vue-router'
 import { useCurrentUser } from 'vuefire'
 
 import { isCloud } from '@/platform/distribution/types'
+import { readCurrentLocationQuery } from '@/platform/navigation/currentLocationQuery'
 
 const STORAGE_KEY = 'Comfy.FeatureFlagOverride'
 const QUERY_PARAM = 'ff'
@@ -46,14 +49,15 @@ function parseOverrideValue(rawValue: string | undefined): unknown {
 }
 
 /**
- * The captured overrides plus the query string they came from. Remembering the
- * source makes capture idempotent: re-reading the same URL does not rewrite
- * storage, so a flag can be read as often as a render needs.
+ * The captured overrides plus the `ff` request they came from. Remembering the
+ * request is what makes capture idempotent across the two entry points: the
+ * lazy first read and the router guard that follows it see the same URL, and
+ * the second one leaves storage alone.
  */
-type StoredState = { search: string; overrides: OverrideMap }
+type StoredState = { request: string; overrides: OverrideMap }
 
 function emptyState(): StoredState {
-  return { search: '', overrides: {} }
+  return { request: '', overrides: {} }
 }
 
 function coerceStoredOverrides(value: unknown): OverrideMap {
@@ -80,10 +84,10 @@ function readStoredState(): StoredState {
   }
   if (typeof parsed !== 'object' || parsed === null) return emptyState()
 
-  const search = 'search' in parsed ? parsed.search : undefined
+  const request = 'request' in parsed ? parsed.request : undefined
   const overrides = 'overrides' in parsed ? parsed.overrides : undefined
   return {
-    search: typeof search === 'string' ? search : '',
+    request: typeof request === 'string' ? request : '',
     overrides: coerceStoredOverrides(overrides)
   }
 }
@@ -102,58 +106,87 @@ function splitRequest(request: string): [name: string, value?: string] {
   return [request.slice(0, separator), request.slice(separator + 1)]
 }
 
-function readOverrideRequests(search: string): string[] {
-  try {
-    return new URLSearchParams(search).getAll(QUERY_PARAM)
-  } catch {
-    return []
-  }
+/**
+ * The `ff` requests in a router query, or undefined when the parameter is
+ * absent — an unrelated URL has to leave an established session alone.
+ *
+ * The router reports a bare `?ff` as null and `?ff=` as an empty string. Both
+ * are the nameless request that clears the session.
+ */
+function readOverrideRequests(query: LocationQuery): string[] | undefined {
+  const raw = query[QUERY_PARAM]
+  if (raw === undefined) return undefined
+
+  const values = Array.isArray(raw) ? raw : [raw]
+  return values.map((value) => value ?? '')
 }
 
 /**
- * Merges `?ff=` requests from the current URL into the overrides already held
- * for this tab. An `?ff=` with no name clears every override in the session.
+ * Merges `?ff=` requests into the overrides already held for this tab. An
+ * `?ff=` with no name clears every override in the session.
  */
-function captureRequests(
+function resolveOverrides(
   requests: string[],
-  stored: OverrideMap,
-  search: string
+  stored: OverrideMap
 ): OverrideMap {
-  if (requests.includes('')) {
-    writeStoredState({ search, overrides: {} })
-    return {}
-  }
+  if (requests.includes('')) return {}
 
   const overrides: OverrideMap = { ...stored }
   for (const request of requests) {
     const [name, rawValue] = splitRequest(request)
     overrides[name] = parseOverrideValue(rawValue)
   }
-
-  writeStoredState({ search, overrides })
   return overrides
 }
 
-function loadSessionOverrides(): OverrideMap {
+/**
+ * Captures the `?ff=` request carried by a router query into this tab's
+ * session. The router guard calls this on every navigation, alongside the
+ * other query-state readers, so the URL is read where all query state is read
+ * rather than on each of the hundreds of flag reads a session performs.
+ */
+export function captureFeatureFlagOverrides(query: LocationQuery): void {
+  if (!isCloud) return
+
+  const requests = readOverrideRequests(query)
+  if (!requests) return
+
+  const request = stringifyQuery({ [QUERY_PARAM]: query[QUERY_PARAM] })
   const stored = readStoredState()
-  const search = window.location.search
-  if (search === stored.search) return stored.overrides
+  if (request === stored.request) return
+
+  writeStoredState({
+    request,
+    overrides: resolveOverrides(requests, stored.overrides)
+  })
+}
 
-  const requests = readOverrideRequests(search)
-  if (requests.length === 0) return stored.overrides
+/**
+ * Flags are read during bootstrap — from `api.init()`, from the auth store's
+ * sign-in path, and from the router's own guards — all of which can run before
+ * the first navigation reaches the guard. The first read therefore captures
+ * the URL itself; every later change of it arrives through the router.
+ */
+let hasCapturedInitialQuery = false
+
+function loadSessionOverrides(): OverrideMap {
+  if (!hasCapturedInitialQuery) {
+    hasCapturedInitialQuery = true
+    captureFeatureFlagOverrides(readCurrentLocationQuery())
+  }
 
-  return captureRequests(requests, stored.overrides, search)
+  return readStoredState().overrides
 }
 
 /**
  * Gets a session override for any feature flag, requested via `?ff=name` to
  * turn it on or `?ff=name:value` for a specific value, repeatable to override
  * several flags at once. A nameless `?ff=` clears the session.
  *
- * The request is captured into `sessionStorage` on the first read, so it
- * survives reloads and in-app navigation but dies when the tab closes. Capture
- * happens before authentication resolves; the employee check is applied here on
- * every read instead, so a flag flips as soon as the user is known.
+ * The request is captured into `sessionStorage`, so it survives reloads and
+ * in-app navigation but dies when the tab closes. Capture happens before
+ * authentication resolves; the employee check is applied here on every read
+ * instead, so a flag flips as soon as the user is known.
  *
  * Returns undefined (not null) as the "no override" sentinel, matching
  * `getDevOverride`.