From 494d00b6fee49b639ca8370aae82ac0291b88f45 Mon Sep 17 00:00:00 2001 From: Kelly Date: Thu, 20 Aug 2026 00:25:11 -0400 Subject: [PATCH 01/12] =?UTF-8?q?=E2=9C=A8=20Add=20remaining=20Datadog=20s?= =?UTF-8?q?ites=20to=20Flags=20tab=20site=20picker?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Expands FLAG_SITES beyond US1 + Staging now that the prod OAuth client has replicated to the other commercial DCs (US3, US5, EU1, AP1, AP2). FED/GovCloud stays excluded since the client isn't registered there. --- .../panel/components/tabs/flagsTab/oauth.spec.ts | 14 +++++++++++++- .../src/panel/components/tabs/flagsTab/oauth.ts | 14 ++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/developer-extension/src/panel/components/tabs/flagsTab/oauth.spec.ts b/developer-extension/src/panel/components/tabs/flagsTab/oauth.spec.ts index fbff50fc5c..5a8b0bdbd2 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/oauth.spec.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/oauth.spec.ts @@ -1,6 +1,7 @@ import { registerCleanupTask, replaceMockable } from '../../../../../../packages/browser-core/test' import { clearStoredTokens, + FLAG_SITES, getFlagsApiHost, getValidAccessToken, loadStoredTokens, @@ -37,11 +38,22 @@ describe('oauth', () => { } describe('getFlagsApiHost', () => { - it('maps each site to its frontend host (US1 → app, staging → dd)', () => { + it('maps each site to its frontend host (US1/EU1 → app, regional → own subdomain, staging → dd)', () => { expect(getFlagsApiHost('datadoghq.com')).toBe('app.datadoghq.com') + expect(getFlagsApiHost('datadoghq.eu')).toBe('app.datadoghq.eu') + expect(getFlagsApiHost('us3.datadoghq.com')).toBe('us3.datadoghq.com') + expect(getFlagsApiHost('us5.datadoghq.com')).toBe('us5.datadoghq.com') + expect(getFlagsApiHost('ap1.datadoghq.com')).toBe('ap1.datadoghq.com') + expect(getFlagsApiHost('ap2.datadoghq.com')).toBe('ap2.datadoghq.com') expect(getFlagsApiHost('datad0g.com')).toBe('dd.datad0g.com') }) + it('covers every site in FLAG_SITES, so a new entry cannot ship without a host', () => { + for (const { site, host } of FLAG_SITES) { + expect(getFlagsApiHost(site)).toBe(host) + } + }) + it('throws on a site that is not in the known list', () => { expect(() => getFlagsApiHost('evil.example')).toThrowError(/Unknown Datadog site/) expect(() => getFlagsApiHost('')).toThrowError(/Unknown Datadog site/) diff --git a/developer-extension/src/panel/components/tabs/flagsTab/oauth.ts b/developer-extension/src/panel/components/tabs/flagsTab/oauth.ts index 8cb78da096..97bb76b940 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/oauth.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/oauth.ts @@ -46,11 +46,21 @@ export interface FlagSite { * The Datadog sites the Flags tab can connect to. A fixed list rather than a free-text host, so * there's no user-entered domain to validate against phishing. * - * Trimmed to US1 + Staging while the prod OAuth client replicates to the other DCs; add them back - * with the same subdomain scheme (US1/EU1 `app.`, staging `dd.`, regional sites as-is). + * Hosts follow the standard scheme: US1 and EU1 are served from `app.`, regional sites from their + * own subdomain, staging from `dd.`. Every non-staging site shares one prod OAuth client (see + * getClientId), which is replicated across the commercial DCs. + * + * FED (`ddog-gov.com`, `us2.ddog-gov.com`) is deliberately absent — GovCloud is a separate + * deployment and the client isn't registered there. Offering a site whose sign-in always fails is + * worse than omitting it. */ export const FLAG_SITES: FlagSite[] = [ { site: 'datadoghq.com', host: 'app.datadoghq.com', label: 'US1 (datadoghq.com)' }, + { site: 'us3.datadoghq.com', host: 'us3.datadoghq.com', label: 'US3 (us3.datadoghq.com)' }, + { site: 'us5.datadoghq.com', host: 'us5.datadoghq.com', label: 'US5 (us5.datadoghq.com)' }, + { site: 'datadoghq.eu', host: 'app.datadoghq.eu', label: 'EU1 (datadoghq.eu)' }, + { site: 'ap1.datadoghq.com', host: 'ap1.datadoghq.com', label: 'AP1 (ap1.datadoghq.com)' }, + { site: 'ap2.datadoghq.com', host: 'ap2.datadoghq.com', label: 'AP2 (ap2.datadoghq.com)' }, { site: 'datad0g.com', host: 'dd.datad0g.com', label: 'Staging (datad0g.com)' }, ] From 37a45fc1e146afe28d418b86124d88f99cdb6e8c Mon Sep 17 00:00:00 2001 From: Kelly Date: Thu, 20 Aug 2026 01:59:43 -0400 Subject: [PATCH 02/12] =?UTF-8?q?=E2=9C=A8=20Flag=20overrides:=20flag=20cr?= =?UTF-8?q?oss-environment=20leftovers,=20add=20signed-out=20Clear=20all?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Overrides live in the inspected page's localStorage, which has no Datadog site scoping, so one created on staging keeps applying on US1. The wrapper can't be changed, so detect it instead: look each overridden key up in the connected site's catalog and mark the row red when no active flag holds the key, or one does but the stored type disagrees. Only a well-formed lookup that matched nothing counts as evidence — a failed request, a still-loading one, or a 2xx whose body isn't the expected envelope must not tell the user to clear a working override. Also adds a Clear all to the signed-out notice (confirm step, error surfacing, and a reload prompt, since the page keeps applying overrides until it rebuilds its provider), and drops a getFlagsApiHost test that passed by construction. Co-Authored-By: Claude Opus 5 (1M context) --- .../tabs/flagsTab/connectScreen.tsx | 91 +++++++++++++++++-- .../tabs/flagsTab/flagCatalogList.tsx | 18 +++- .../tabs/flagsTab/flagTypes.spec.ts | 22 ++++- .../components/tabs/flagsTab/flagTypes.ts | 13 +++ .../components/tabs/flagsTab/flagsContext.tsx | 5 +- .../tabs/flagsTab/flagsRequests.spec.ts | 22 ++++- .../components/tabs/flagsTab/flagsRequests.ts | 47 +++++++--- .../components/tabs/flagsTab/oauth.spec.ts | 7 -- .../tabs/flagsTab/useOverriddenFlags.spec.ts | 32 +++++-- .../tabs/flagsTab/useOverriddenFlags.ts | 33 ++++--- 10 files changed, 235 insertions(+), 55 deletions(-) diff --git a/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx b/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx index 2b630c8751..ee0290cefa 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx +++ b/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx @@ -1,5 +1,6 @@ import { Alert, Badge, Box, Button, Center, Group, Select, Stack, Text } from '@mantine/core' -import React from 'react' +import React, { useEffect, useRef, useState } from 'react' +import { toErrorMessage } from '../../../../common/toErrorMessage' import { useSettings } from '../../../hooks/useSettings' import type { FlagAuthState } from './useFlagAuth' import { useInspectedPageOverrides } from './useInspectedPageOverrides' @@ -41,17 +42,64 @@ export function ConnectScreen({ auth }: { auth: FlagAuthState }) { } /** - * Surfaces overrides already stored on the inspected page while signed out — otherwise this screen is - * all that renders, so an override left from an earlier session keeps affecting the page with nothing - * to explain it. Informational only; everything that mutates overrides lives on the connected tab. + * Surfaces overrides already stored on the inspected page while signed out, with a Clear all so they + * can be wiped without signing in just for that. * * Mounted only while disconnected, so its navigation listeners never run alongside the connected * tab's own instance of this hook. */ function DisconnectedOverridesNotice() { - const { status, overrides } = useInspectedPageOverrides() + const { status, overrides, clearAll, reloadPage } = useInspectedPageOverrides() + const [phase, setPhase] = useState<'idle' | 'confirming' | 'clearing' | 'cleared'>('idle') + const [error, setError] = useState(null) + // Bumped on navigation so a clear that settles afterwards can't report on the page it left. + const generation = useRef(0) const count = Object.keys(overrides).length + // A navigation brings a fresh set of overrides: a pending confirmation would clear those instead, + // and a previous "cleared" no longer describes the page. + useEffect(() => { + if (status === 'loading') { + generation.current += 1 + setPhase('idle') + setError(null) + } + }, [status]) + + const handleClearAll = () => { + const started = generation.current + setPhase('clearing') + setError(null) + void clearAll() + .then(() => { + if (started === generation.current) { + setPhase('cleared') + } + }) + .catch((err: unknown) => { + if (started === generation.current) { + setError(toErrorMessage(err)) + setPhase('idle') + } + }) + } + + // Gated on the page agreeing it has none: a same-document SPA navigation fires no webNavigation + // event, so without this the banner could outlive the state it describes. + if (phase === 'cleared' && status === 'ready' && count === 0) { + // The page keeps applying them until it reloads and rebuilds its provider. + return ( + + + Reload the page to stop applying them. + + + + ) + } + if (status !== 'ready' || count === 0) { return null } @@ -66,8 +114,39 @@ function DisconnectedOverridesNotice() { title={`${count} override${count === 1 ? '' : 's'} active on this page`} > - These are stored in the page and keep applying while you are signed out. Sign in to view and remove them. + These are stored in the page and keep applying while you are signed out. Sign in to review and remove them + individually. + + {phase === 'confirming' || phase === 'clearing' ? ( + <> + + Clear all {count} override{count === 1 ? '' : 's'} on this page? + + + + + ) : ( + + )} + + {error && ( + + Could not clear the overrides: {error} + + )} ) } diff --git a/developer-extension/src/panel/components/tabs/flagsTab/flagCatalogList.tsx b/developer-extension/src/panel/components/tabs/flagsTab/flagCatalogList.tsx index 99ece77997..feee5692df 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/flagCatalogList.tsx +++ b/developer-extension/src/panel/components/tabs/flagsTab/flagCatalogList.tsx @@ -16,7 +16,7 @@ import { IconArrowBackUp, IconCopy } from '@tabler/icons-react' import React, { useLayoutEffect, useRef, useState, type ReactNode } from 'react' import type { CatalogFlag } from './flagsRequests' import { useFlagsContext } from './flagsContext' -import { validateOverrideValue } from './flagTypes' +import { flagTypeLabel, isOverrideUnusable, validateOverrideValue } from './flagTypes' import { getOverride, type FlagOverride } from './inspectedPageFlags' export function FlagCatalogBody() { @@ -121,6 +121,7 @@ function FlagRow({ onRevert: (flagKey: string) => void }) { const overridden = override !== undefined + const unusable = isOverrideUnusable(flag, override) return ( @@ -141,6 +146,13 @@ function FlagRow({ {flag.description && } + {override && unusable && ( + + {flag.unresolved + ? "No active flag with this key on this site — it's from a different Datadog environment, or archived here. Clear it." + : `Type mismatch: stored as ${flagTypeLabel(override.type)}, this flag is ${flagTypeLabel(flag.type)}. Clear it.`} + + )} {overridden && ( @@ -156,7 +168,7 @@ function FlagRow({ ) : ( flag.variants.map((variant) => { - const isActive = overridden && valuesEqual(override.value, variant.value) + const isActive = overridden && !unusable && valuesEqual(override.value, variant.value) // The catalog keeps an unparseable variant as its raw string (see parseVariantValue), and // writing that through would break the override type contract. `allowNull` keeps a // legitimate JSON `null` variant applyable. diff --git a/developer-extension/src/panel/components/tabs/flagsTab/flagTypes.spec.ts b/developer-extension/src/panel/components/tabs/flagsTab/flagTypes.spec.ts index 376d247e1e..13cf339675 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/flagTypes.spec.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/flagTypes.spec.ts @@ -1,4 +1,24 @@ -import { flagTypeLabel, validateOverrideValue, type FlagType } from './flagTypes' +import { flagTypeLabel, isOverrideUnusable, validateOverrideValue, type FlagType } from './flagTypes' + +describe('isOverrideUnusable', () => { + const flag = { type: 'BOOLEAN' } as const + + it('is false with no override', () => { + expect(isOverrideUnusable(flag, undefined)).toBe(false) + }) + + it('is false when the override matches the resolved flag', () => { + expect(isOverrideUnusable(flag, { type: 'BOOLEAN' })).toBe(false) + }) + + it('is true when no flag on this site holds the key', () => { + expect(isOverrideUnusable({ ...flag, unresolved: true }, { type: 'BOOLEAN' })).toBe(true) + }) + + it('is true when the stored type disagrees with the resolved flag', () => { + expect(isOverrideUnusable(flag, { type: 'JSON' })).toBe(true) + }) +}) describe('flagTypeLabel', () => { it('returns the display label, or the raw type for an unsupported value_type', () => { diff --git a/developer-extension/src/panel/components/tabs/flagsTab/flagTypes.ts b/developer-extension/src/panel/components/tabs/flagsTab/flagTypes.ts index 7611ea848c..534bcef40f 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/flagTypes.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/flagTypes.ts @@ -64,6 +64,19 @@ export function parseTypedString(type: Exclude, raw: string } } +/** + * Flags an override the connected site can't honour: no active flag holds the key, or one does but + * the stored type disagrees. localStorage has no site concept, so the usual cause is an override + * left by another Datadog site — though a flag archived here, or a manual override applied with the + * wrong type, land here too, and a key that exists identically on both sites is missed entirely. + */ +export function isOverrideUnusable( + flag: { type: FlagType; unresolved?: boolean }, + override: { type: FlagType } | undefined +): boolean { + return override !== undefined && (flag.unresolved === true || override.type !== flag.type) +} + /** * Validates an already-parsed override value against its declared type, returning an error message * or null. The value-level counterpart to parseTypedString: the catalog's variant-click path diff --git a/developer-extension/src/panel/components/tabs/flagsTab/flagsContext.tsx b/developer-extension/src/panel/components/tabs/flagsTab/flagsContext.tsx index 8797854ad0..2508116135 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/flagsContext.tsx +++ b/developer-extension/src/panel/components/tabs/flagsTab/flagsContext.tsx @@ -72,7 +72,7 @@ export function FlagsProvider({ auth, children }: { auth: FlagAuthState; childre // Fetched by key so an overridden flag shows regardless of which catalog page it's on, with a // minimal row as fallback for a key that no longer resolves (so it can still be reverted). const overrideKeys = useMemo(() => Object.keys(overrides), [overrides]) - const overriddenCatalogFlags = useOverriddenFlags(auth, overrideKeys) + const { flags: overriddenCatalogFlags, missingKeys } = useOverriddenFlags(auth, overrideKeys) const overriddenFlags = useMemo( () => overrideKeys.map( @@ -84,9 +84,10 @@ export function FlagsProvider({ auth, children }: { auth: FlagAuthState; childre type: overrides[key].type, variants: [], tags: [], + unresolved: missingKeys.has(key), } ), - [overrideKeys, overriddenCatalogFlags, overrides] + [overrideKeys, overriddenCatalogFlags, missingKeys, overrides] ) // Dropped from the paginated list so they don't show twice. const bottomFlags = useMemo( diff --git a/developer-extension/src/panel/components/tabs/flagsTab/flagsRequests.spec.ts b/developer-extension/src/panel/components/tabs/flagsTab/flagsRequests.spec.ts index 07c55070ab..d656a961f3 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/flagsRequests.spec.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/flagsRequests.spec.ts @@ -247,22 +247,35 @@ describe('flagsRequests', () => { return Promise.resolve(new Response(JSON.stringify({ data }))) }) - const flags = await fetchFlagsByKeys('tok', 'datad0g.com', ['flag-a', 'missing', 'flag-b']) + const { flags, missingKeys } = await fetchFlagsByKeys('tok', 'datad0g.com', ['flag-a', 'missing', 'flag-b']) expect(spy).toHaveBeenCalledTimes(3) const firstUrl = new URL(spy.calls.argsFor(0)[0] as string) expect(firstUrl.searchParams.get('key')).toBe('flag-a') + // Active-only: an archived flag sharing the key would win the dedupe and describe the override + // against the wrong type and variants. expect(firstUrl.searchParams.get('is_archived')).toBe('false') expect(flags.map((flag) => flag.key)).toEqual(['flag-a', 'flag-b']) + expect(missingKeys).toEqual(['missing']) + }) + + it('does not call a key missing when the response body has no data array', async () => { + // A 2xx that isn't the expected envelope (proxy interstitial, schema change) is tolerated so + // the section still renders, but it is not evidence the flag is gone. + spyOn(globalThis, 'fetch').and.returnValue(Promise.resolve(new Response(JSON.stringify({ errors: ['x'] })))) + + const { flags, missingKeys } = await fetchFlagsByKeys('tok', 'datad0g.com', ['flag-a']) + expect(flags).toEqual([]) + expect(missingKeys).toEqual([]) }) it('makes no request and returns nothing for an empty key list', async () => { const spy = spyOn(globalThis, 'fetch') - expect(await fetchFlagsByKeys('tok', 'datad0g.com', [])).toEqual([]) + expect(await fetchFlagsByKeys('tok', 'datad0g.com', [])).toEqual({ flags: [], missingKeys: [] }) expect(spy).not.toHaveBeenCalled() }) - it('drops a key whose request fails but keeps the ones that resolve', async () => { + it('drops a key whose request fails, keeps the ones that resolve, and does not call it missing', async () => { spyOn(globalThis, 'fetch').and.callFake((input) => { const key = new URL(input as string).searchParams.get('key') if (key === 'boom') { @@ -273,8 +286,9 @@ describe('flagsRequests', () => { ) }) - const flags = await fetchFlagsByKeys('tok', 'datad0g.com', ['flag-a', 'boom', 'flag-b']) + const { flags, missingKeys } = await fetchFlagsByKeys('tok', 'datad0g.com', ['flag-a', 'boom', 'flag-b']) expect(flags.map((flag) => flag.key)).toEqual(['flag-a', 'flag-b']) + expect(missingKeys).toEqual([]) }) }) }) diff --git a/developer-extension/src/panel/components/tabs/flagsTab/flagsRequests.ts b/developer-extension/src/panel/components/tabs/flagsTab/flagsRequests.ts index 729dd92a54..8989712614 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/flagsRequests.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/flagsRequests.ts @@ -12,6 +12,8 @@ export interface CatalogFlag { tags: string[] /** Undefined for flags created by a service account or integration, which carry no user UUID. */ createdBy?: string + /** Synthesized locally, never from the API: no active flag on the connected site holds this key. */ + unresolved?: boolean } /** @@ -113,42 +115,65 @@ export function fetchFlagCatalog(token: string, site: string, request: FlagCatal return fetchFlagPage(url, token, 'Failed to fetch flag catalog') } +export interface FlagsByKeysResult { + flags: CatalogFlag[] + /** Keys whose lookup completed and matched nothing; a failed lookup is not in here. */ + missingKeys: string[] +} + /** * Fetches a specific set of flags by exact key, one request per key (the endpoint's `key` filter is * exact and single-valued — there's no batched lookup). Used for the "Local overrides" section, * which must show overridden flags even when they're not on the current catalog page. * * Settles per key rather than Promise.all: one key failing transiently must not discard the flags - * that did resolve, or the whole section collapses to bare fallback rows. Keys with no match - * (deleted, or an override for a non-existent flag) are simply absent, and the caller falls back to - * a minimal row. Error labels omit the key — it's customer data, kept out of logs. + * that did resolve, or the whole section collapses to bare fallback rows. Only a well-formed + * response that matched nothing counts as `missingKeys` — a failed request, or a 2xx whose body + * isn't the expected envelope, proves nothing. Error labels omit the key — it's customer data. + * + * Active-only for the same reason as the catalog: an archived and an active flag can share a key, + * and mapResources keeps whichever the server listed first, so including archived ones would risk + * describing the override against the wrong flag's type and variants. The cost is that an override + * on a flag archived here reads as absent, which the row copy accounts for. */ -export async function fetchFlagsByKeys(token: string, site: string, keys: string[]): Promise { +export async function fetchFlagsByKeys(token: string, site: string, keys: string[]): Promise { const host = getFlagsApiHost(site) const results = await Promise.allSettled( - keys.map(async (key) => { + keys.map((key) => { const url = new URL(`https://${host}/api/ui/ffe/feature-flags`) url.searchParams.set('key', key) url.searchParams.set('is_archived', 'false') - const { flags } = await fetchFlagPage(url, token, 'Failed to fetch flag') - return flags + return fetchFlagPage(url, token, 'Failed to fetch flag') }) ) - return results.flatMap((result) => (result.status === 'fulfilled' ? result.value : [])) + const flags = results.flatMap((result) => (result.status === 'fulfilled' ? result.value.flags : [])) + const foundKeys = new Set(flags.map((flag) => flag.key)) + const missingKeys = keys.filter( + (key, index) => results[index].status === 'fulfilled' && results[index].value.wellFormed && !foundKeys.has(key) + ) + return { flags, missingKeys } } /** * Shared request/response handling for both fetch functions: run the request, tolerate a response * that omits or mistypes `data`, and map its resources. `total` falls back to the resource count * when the server omits `meta.page.total` (a partial response, or the by-key lookup which sends no - * pagination fields). + * pagination fields). `wellFormed` reports whether `data` was actually there, so a caller reading + * meaning into an empty result can tell a real no-match from a body we merely tolerated. */ -async function fetchFlagPage(url: URL, token: string, errorLabel: string): Promise { +async function fetchFlagPage( + url: URL, + token: string, + errorLabel: string +): Promise { const body = await fetchFfeJson(url.toString(), token, errorLabel) - const resources = Array.isArray(body?.data) ? body.data : [] + const data = body?.data + const wellFormed = Array.isArray(data) + const resources = wellFormed ? data : [] return { flags: mapResources(resources), total: body?.meta?.page?.total ?? resources.length, + wellFormed, } } diff --git a/developer-extension/src/panel/components/tabs/flagsTab/oauth.spec.ts b/developer-extension/src/panel/components/tabs/flagsTab/oauth.spec.ts index 5a8b0bdbd2..f2cde2d266 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/oauth.spec.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/oauth.spec.ts @@ -1,7 +1,6 @@ import { registerCleanupTask, replaceMockable } from '../../../../../../packages/browser-core/test' import { clearStoredTokens, - FLAG_SITES, getFlagsApiHost, getValidAccessToken, loadStoredTokens, @@ -48,12 +47,6 @@ describe('oauth', () => { expect(getFlagsApiHost('datad0g.com')).toBe('dd.datad0g.com') }) - it('covers every site in FLAG_SITES, so a new entry cannot ship without a host', () => { - for (const { site, host } of FLAG_SITES) { - expect(getFlagsApiHost(site)).toBe(host) - } - }) - it('throws on a site that is not in the known list', () => { expect(() => getFlagsApiHost('evil.example')).toThrowError(/Unknown Datadog site/) expect(() => getFlagsApiHost('')).toThrowError(/Unknown Datadog site/) diff --git a/developer-extension/src/panel/components/tabs/flagsTab/useOverriddenFlags.spec.ts b/developer-extension/src/panel/components/tabs/flagsTab/useOverriddenFlags.spec.ts index 48cb09932c..648f039ec6 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/useOverriddenFlags.spec.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/useOverriddenFlags.spec.ts @@ -1,9 +1,8 @@ import React, { act } from 'react' import { createRoot } from 'react-dom/client' import { registerCleanupTask } from '../../../../../../packages/browser-core/test' -import type { CatalogFlag } from './flagsRequests' import type { FlagAuthState } from './useFlagAuth' -import { useOverriddenFlags } from './useOverriddenFlags' +import { useOverriddenFlags, type OverriddenFlagsState } from './useOverriddenFlags' // useOverriddenFlags only reads isConnected + site off the auth object. const AUTH = { isConnected: true, site: 'datad0g.com' } as FlagAuthState @@ -28,7 +27,7 @@ describe('useOverriddenFlags', () => { function mountHook(keys: string[]) { const container = document.createElement('div') const root = createRoot(container) - let latest: CatalogFlag[] = [] + let latest: OverriddenFlagsState = { flags: [], missingKeys: new Set() } function Probe() { latest = useOverriddenFlags(AUTH, keys) return null @@ -57,7 +56,22 @@ describe('useOverriddenFlags', () => { await flush() expect(fetchSpy).toHaveBeenCalledTimes(2) - expect(get().map((flag) => flag.key)).toEqual(jasmine.arrayWithExactContents(['flag-a', 'flag-b'])) + expect(get().flags.map((flag) => flag.key)).toEqual(jasmine.arrayWithExactContents(['flag-a', 'flag-b'])) + expect(get().missingKeys.size).toBe(0) + }) + + it('reports a key the catalog has no match for as missing', async () => { + spyOn(globalThis, 'fetch').and.callFake((input: RequestInfo | URL) => { + const key = new URL(input as string).searchParams.get('key') + const data = key === 'gone' ? [] : [{ attributes: { key, name: `Name ${key}`, value_type: 'STRING' } }] + return Promise.resolve(new Response(JSON.stringify({ data }))) + }) + + const get = mountHook(['flag-a', 'gone']) + await flush() + + expect(get().flags.map((flag) => flag.key)).toEqual(['flag-a']) + expect(Array.from(get().missingKeys)).toEqual(['gone']) }) it('does not fetch when there are no overridden keys', async () => { @@ -65,18 +79,16 @@ describe('useOverriddenFlags', () => { const get = mountHook([]) await flush() expect(fetchSpy).not.toHaveBeenCalled() - expect(get()).toEqual([]) + expect(get().flags).toEqual([]) }) - it('returns no flags when a key lookup fails (non-blocking)', async () => { - // The failure is expected here; the hook logs it via console.error — swallow it so the CI - // unexpected-error-log reporter doesn't flag the intentional log. - spyOn(console, 'error') + it('does not report a key as missing when its lookup failed', async () => { spyOn(globalThis, 'fetch').and.returnValue( Promise.resolve(new Response('nope', { status: 500, statusText: 'Server Error' })) ) const get = mountHook(['flag-a']) await flush() - expect(get()).toEqual([]) + expect(get().flags).toEqual([]) + expect(get().missingKeys.size).toBe(0) }) }) diff --git a/developer-extension/src/panel/components/tabs/flagsTab/useOverriddenFlags.ts b/developer-extension/src/panel/components/tabs/flagsTab/useOverriddenFlags.ts index 8e83733315..3354dbd51a 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/useOverriddenFlags.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/useOverriddenFlags.ts @@ -7,15 +7,24 @@ import type { FlagAuthState } from './useFlagAuth' const logger = createLogger('useOverriddenFlags') +export interface OverriddenFlagsState { + flags: CatalogFlag[] + /** Keys a well-formed lookup proved absent. Empty while loading and after a failure. */ + missingKeys: ReadonlySet +} + +const EMPTY: OverriddenFlagsState = { flags: [], missingKeys: new Set() } + /** * Loads the catalog data (name, variants, type) for the currently-overridden flag keys, so the - * "Local overrides" section can render them even when they're not on the current catalog page. Keys - * that don't resolve to a flag are simply absent — the caller falls back to a minimal row so every - * override can still be reverted. Failures are non-blocking (the section just shows fallback rows). + * "Local overrides" section can render them even when they're not on the current catalog page. A key + * that doesn't resolve falls back to a minimal row so the override can still be reverted, and lands + * in `missingKeys` — which the row turns into a "clear this" warning, so only proven absence counts. + * Failures are non-blocking: the section shows fallback rows, unmarked. */ -export function useOverriddenFlags(auth: FlagAuthState, keys: string[]): CatalogFlag[] { +export function useOverriddenFlags(auth: FlagAuthState, keys: string[]): OverriddenFlagsState { const { isConnected, site } = auth - const [flags, setFlags] = useState([]) + const [state, setState] = useState(EMPTY) // Sort + join so the effect only reruns when the *set* of keys changes, not on every render (the // caller passes a fresh array each time). @@ -24,22 +33,24 @@ export function useOverriddenFlags(auth: FlagAuthState, keys: string[]): Catalog useEffect(() => { const keyArray = keyList ? keyList.split('\n') : [] if (!isConnected || keyArray.length === 0) { - setFlags([]) + setState(EMPTY) return } let cancelled = false + // Drop the previous verdict while the new lookup runs; keep the flags so rows don't blank out. + setState((previous) => ({ flags: previous.flags, missingKeys: new Set() })) getValidAccessToken(site) - .then((token) => (token ? fetchFlagsByKeys(token, site, keyArray) : [])) - .then((loaded) => { + .then((token) => (token ? fetchFlagsByKeys(token, site, keyArray) : { flags: [], missingKeys: [] })) + .then(({ flags, missingKeys }) => { if (!cancelled) { - setFlags(loaded) + setState({ flags, missingKeys: new Set(missingKeys) }) } }) .catch((err: unknown) => { if (!cancelled) { logger.error('Error while fetching overridden flags:', err) - setFlags([]) + setState(EMPTY) } }) @@ -48,5 +59,5 @@ export function useOverriddenFlags(auth: FlagAuthState, keys: string[]): Catalog } }, [isConnected, site, keyList]) - return flags + return state } From 4fe2a769f319d411d9cf09cbbd6fdaf25bf0729a Mon Sep 17 00:00:00 2001 From: Kelly Date: Thu, 20 Aug 2026 03:43:12 -0400 Subject: [PATCH 03/12] =?UTF-8?q?=E2=9C=A8=20Scope=20flag=20overrides=20pe?= =?UTF-8?q?r=20Datadog=20site?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Overrides live in the inspected page's localStorage under a single key the wrapper reads on init, with no site in it. localStorage is scoped per browser origin, not per Datadog site, so an override set on staging kept applying — invisibly — after switching to US1. We can't change the wrapper. So the extension keeps its own per-site stores and projects the connected site's into the key the wrapper reads. Staging's copy is parked rather than sitting where the wrapper can see it, which fixes the case detection never could: the same key existing in both sites with the same type. Projection is a no-op when nothing would change, so signing in stays inert and a reload is only ever demanded when the page really is running another site's values. Pre-scoping overrides are adopted once, gated on no site store holding anything, so no site inherits another's. A failed projection is surfaced rather than assumed to have worked. The cross-environment warning goes with it — an override in a site's store is that site's by construction. The row still notes a flag archived or deleted since the override was set, and still flags a type mismatch the wrapper would reject, now split by severity. Co-Authored-By: Claude Opus 5 (1M context) --- .../tabs/flagsTab/connectScreen.tsx | 4 +- .../tabs/flagsTab/flagCatalogList.tsx | 23 ++- .../tabs/flagsTab/flagTypes.spec.ts | 22 +-- .../components/tabs/flagsTab/flagTypes.ts | 13 -- .../components/tabs/flagsTab/flagsContext.tsx | 22 ++- .../components/tabs/flagsTab/flagsRequests.ts | 4 +- .../components/tabs/flagsTab/flagsTab.tsx | 37 ++++- .../tabs/flagsTab/inspectedPageFlags.spec.ts | 110 ++++++++++++++ .../tabs/flagsTab/inspectedPageFlags.ts | 139 ++++++++++++++++-- .../flagsTab/useInspectedPageOverrides.ts | 79 ++++++++-- .../tabs/flagsTab/useOverriddenFlags.ts | 4 +- 11 files changed, 383 insertions(+), 74 deletions(-) diff --git a/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx b/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx index ee0290cefa..d5e604beab 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx +++ b/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx @@ -120,8 +120,10 @@ function DisconnectedOverridesNotice() { {phase === 'confirming' || phase === 'clearing' ? ( <> + {/* Signed out there's no site to scope to, so this wipes the saved stores for every + Datadog site as well — say so rather than deleting more than the count implies. */} - Clear all {count} override{count === 1 ? '' : 's'} on this page? + Clear all overrides on this page, including any saved for other Datadog sites? + + + + + )} + + {scopeError && ( + <> + + {scopeError} + + + + )} + {mutationError && ( <> diff --git a/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.spec.ts b/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.spec.ts index 3a2d26bb56..2d8c1147fb 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.spec.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.spec.ts @@ -5,9 +5,27 @@ import { clearAllOverrides, deleteOverride, readFlagState, + siteOverridesKey, + syncSiteOverrides, writeOverride, } from './inspectedPageFlags' +const STAGING = 'datad0g.com' +const US1 = 'datadoghq.com' + +function clearSiteStores() { + for (let i = localStorage.length - 1; i >= 0; i--) { + const key = localStorage.key(i) + if (key?.startsWith(`${OVERRIDES_KEY}.`)) { + localStorage.removeItem(key) + } + } +} + +function stored(key: string): unknown { + return JSON.parse(localStorage.getItem(key) || 'null') +} + describe('inspectedPageFlags read/write against page localStorage', () => { beforeEach(() => { // Karma runs in a real browser, so evalInWindow's code can be evaluated directly @@ -29,10 +47,12 @@ describe('inspectedPageFlags read/write against page localStorage', () => { } localStorage.removeItem(OVERRIDES_KEY) localStorage.removeItem(DEVTOOLS_MARKER_KEY) + clearSiteStores() registerCleanupTask(() => { ;(globalThis as any).chrome = previousChrome localStorage.removeItem(OVERRIDES_KEY) localStorage.removeItem(DEVTOOLS_MARKER_KEY) + clearSiteStores() }) }) @@ -109,4 +129,94 @@ describe('inspectedPageFlags read/write against page localStorage', () => { expect(Object.prototype.hasOwnProperty.call(storedValue, '__proto__')).toBe(true) expect(storedValue['__proto__']).toEqual({ enabled: true }) }) + + describe('site scoping', () => { + const override = { type: 'BOOLEAN', value: true } as const + + it("keeps a site's overrides in its own store and mirrors them into the key the wrapper reads", async () => { + await writeOverride('dark-mode', override, STAGING) + + expect(stored(siteOverridesKey(STAGING))).toEqual({ 'dark-mode': override }) + expect(stored(OVERRIDES_KEY)).toEqual({ 'dark-mode': override }) + }) + + it('reads the connected site when given one, and what the page is applying when not', async () => { + await writeOverride('dark-mode', override, STAGING) + await syncSiteOverrides(US1) + + // US1 has none of its own; signed out we report the projection, which is now US1's (empty). + expect((await readFlagState(STAGING))?.overrides).toEqual({ 'dark-mode': override }) + expect((await readFlagState(US1))?.overrides).toEqual({}) + expect((await readFlagState())?.overrides).toEqual({}) + }) + + it("stops one site's override from applying on another", async () => { + await writeOverride('dark-mode', override, STAGING) + + const result = await syncSiteOverrides(US1) + + // The page would otherwise still be running staging's value under US1. + expect(stored(OVERRIDES_KEY)).toEqual({}) + expect(result?.changed).toBe(true) + // Staging keeps its own copy, so switching back restores it. + expect(stored(siteOverridesKey(STAGING))).toEqual({ 'dark-mode': override }) + }) + + it('reports no change when the page is already running this site, so no reload is demanded', async () => { + await writeOverride('dark-mode', override, STAGING) + expect((await syncSiteOverrides(STAGING))?.changed).toBe(false) + }) + + it('adopts pre-scoping overrides into the first site connected, without disturbing the page', async () => { + localStorage.setItem(OVERRIDES_KEY, JSON.stringify({ legacy: override })) + + const result = await syncSiteOverrides(STAGING) + + expect(stored(siteOverridesKey(STAGING))).toEqual({ legacy: override }) + expect(result?.changed).toBe(false) + }) + + it('does not adopt again once a site store holds something, or every site would inherit the last one', async () => { + await writeOverride('dark-mode', override, STAGING) + localStorage.setItem(OVERRIDES_KEY, JSON.stringify({ 'dark-mode': override })) + + await syncSiteOverrides(US1) + + expect(stored(OVERRIDES_KEY)).toEqual({}) + expect(localStorage.getItem(siteOverridesKey(US1))).toBeNull() + }) + + it('writes no store for a site with nothing in it, so adoption stays available on a clean page', async () => { + await syncSiteOverrides(US1) + expect(localStorage.getItem(siteOverridesKey(US1))).toBeNull() + + // An override written straight to the wrapper's key afterwards is adopted, not wiped. + localStorage.setItem(OVERRIDES_KEY, JSON.stringify({ legacy: override })) + await syncSiteOverrides(US1) + + expect(stored(OVERRIDES_KEY)).toEqual({ legacy: override }) + expect(stored(siteOverridesKey(US1))).toEqual({ legacy: override }) + }) + + it("clearing while connected leaves the other sites' overrides alone", async () => { + await writeOverride('dark-mode', override, STAGING) + await writeOverride('other-flag', override, US1) + + await clearAllOverrides(US1) + + expect(stored(siteOverridesKey(US1))).toEqual({}) + expect(stored(siteOverridesKey(STAGING))).toEqual({ 'dark-mode': override }) + }) + + it('clearing while signed out wipes every store, so nothing reappears on reconnect', async () => { + await writeOverride('dark-mode', override, STAGING) + await writeOverride('other-flag', override, US1) + + await clearAllOverrides() + + expect(stored(OVERRIDES_KEY)).toEqual({}) + expect(localStorage.getItem(siteOverridesKey(STAGING))).toBeNull() + expect(localStorage.getItem(siteOverridesKey(US1))).toBeNull() + }) + }) }) diff --git a/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts b/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts index 3bc0b74a3c..7b1d8bbca9 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts @@ -10,6 +10,21 @@ const logger = createLogger('inspectedPageFlags') export const OVERRIDES_KEY = 'dd.dd_flag.overrides' export const DEVTOOLS_MARKER_KEY = 'dd.dd_flag.devtools' +/** + * Per-site stores, owned by this extension alone. The wrapper knows nothing about them: it only ever + * reads OVERRIDES_KEY, which we keep as a projection of the connected site's store. That's what + * stops an override made on staging from applying on US1 — the other site's copy is parked here + * rather than sitting in the key the wrapper reads. + * + * The trailing dot matters: it keeps these keys from colliding with OVERRIDES_KEY itself when we + * enumerate them. + */ +const SITE_OVERRIDES_PREFIX = `${OVERRIDES_KEY}.` + +export function siteOverridesKey(site: string): string { + return SITE_OVERRIDES_PREFIX + site +} + export interface FlagOverride { type: FlagType /** @@ -55,19 +70,26 @@ export function sanitizeOverrides(overrides: Record): FlagOverr return sanitized } -// Shared prelude for every inspected-window eval: parses the overrides map from localStorage, +// Shared prelude for every inspected-window eval: parses an overrides map out of localStorage, // tolerating malformed/absent/mistyped storage, and leaves a normalized `overrides` in scope. // Defined once so the read and mutation paths can't interpret storage differently as the contract -// evolves. -const READ_OVERRIDES_PRELUDE = ` +// evolves. `storeKey` is the site's store when connected, and OVERRIDES_KEY when signed out. +function readOverridesPrelude(storeKey: string): string { + return ` let overrides = {} try { - const parsed = JSON.parse(localStorage.getItem(${JSON.stringify(OVERRIDES_KEY)}) || '{}') + const parsed = JSON.parse(localStorage.getItem(${JSON.stringify(storeKey)}) || '{}') if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { overrides = parsed } } catch (e) {} ` +} + +/** Key order can differ between two equal maps, so compare entries rather than raw JSON. */ +const STABLE_STRINGIFY = ` + const stable = (map) => JSON.stringify(Object.keys(map).sort().map((k) => [k, map[k]])) +` /** * Reads the current overrides and enablement marker straight from the inspected page's localStorage @@ -77,10 +99,13 @@ const READ_OVERRIDES_PRELUDE = ` * so the caller keeps its last good values instead of blanking the overrides and flashing the * "not detected" warning. */ -export async function readFlagState(): Promise { +export async function readFlagState(site?: string): Promise { + // Connected, so show that site's own overrides. Signed out there's no site to scope by, and what + // matters is what the page is actually applying — which is the projection. + const storeKey = site ? siteOverridesKey(site) : OVERRIDES_KEY try { const raw = (await evalInWindow(` - ${READ_OVERRIDES_PRELUDE} + ${readOverridesPrelude(storeKey)} const devtoolsEnabled = localStorage.getItem(${JSON.stringify(DEVTOOLS_MARKER_KEY)}) === 'enabled' return { overrides, devtoolsEnabled } `)) as FlagState @@ -91,34 +116,118 @@ export async function readFlagState(): Promise { } } +/** + * Points the key the wrapper reads at `site`'s store, so only that site's overrides apply. Returns + * that store, plus whether the projection actually changed: if it didn't, the page is already + * running the right values and must not be asked to reload — that's what keeps signing in inert. + * + * Also adopts pre-scoping overrides, but only while no site store holds anything. Adopting per-site + * would copy whatever is live into each site as you visit it, which is the leak this exists to stop. + * Once a store does hold something the extension owns the projection, and anything written straight + * to OVERRIDES_KEY from outside is overwritten on the next sync. + * + * Returns null if the page couldn't be written to. The caller must surface that rather than assume + * success — an unprojected page keeps applying whichever site it had. + */ +export async function syncSiteOverrides(site: string): Promise<{ changed: boolean; overrides: FlagOverrides } | null> { + const storeKey = siteOverridesKey(site) + try { + const result = (await evalInWindow(` + ${STABLE_STRINGIFY} + const parse = (raw) => { + try { + const parsed = JSON.parse(raw || 'null') + return parsed && typeof parsed === 'object' && !Array.isArray(parsed) ? parsed : null + } catch (e) { + return null + } + } + + const projection = parse(localStorage.getItem(${JSON.stringify(OVERRIDES_KEY)})) || {} + let siteOverrides = parse(localStorage.getItem(${JSON.stringify(storeKey)})) + + if (siteOverrides === null) { + let anySiteOverrides = false + for (let i = 0; i < localStorage.length; i++) { + const key = localStorage.key(i) + if (key && key.indexOf(${JSON.stringify(SITE_OVERRIDES_PREFIX)}) === 0) { + const store = parse(localStorage.getItem(key)) + if (store && Object.keys(store).length > 0) { + anySiteOverrides = true + break + } + } + } + siteOverrides = anySiteOverrides ? {} : projection + // Only persist a store with something in it. Writing an empty one on a clean page would + // disarm adoption for this origin forever, so a later hand-written override would be wiped. + if (Object.keys(siteOverrides).length > 0) { + localStorage.setItem(${JSON.stringify(storeKey)}, JSON.stringify(siteOverrides)) + } + } + + const changed = stable(projection) !== stable(siteOverrides) + if (changed) { + localStorage.setItem(${JSON.stringify(OVERRIDES_KEY)}, JSON.stringify(siteOverrides)) + } + return { changed, overrides: siteOverrides } + `)) as { changed: boolean; overrides: Record } + return { changed: result.changed, overrides: sanitizeOverrides(result.overrides ?? {}) } + } catch (error) { + logger.error('Error while scoping flag overrides to the site:', error) + return null + } +} + /** * Reads, mutates, and writes back the overrides map in a single round trip, returning the resulting * map so the caller needs no follow-up read. Kept as one eval because splitting it would let a page * navigation land between read and write, applying the previous origin's overrides to the new one. */ -async function applyOverrideStatement(statement: string): Promise> { +async function applyOverrideStatement(statement: string, site?: string): Promise> { + const storeKey = site ? siteOverridesKey(site) : OVERRIDES_KEY return (await evalInWindow(` - ${READ_OVERRIDES_PRELUDE} + ${readOverridesPrelude(storeKey)} ${statement} - localStorage.setItem(${JSON.stringify(OVERRIDES_KEY)}, JSON.stringify(overrides)) + const serialized = JSON.stringify(overrides) + localStorage.setItem(${JSON.stringify(storeKey)}, serialized) + ${site ? `localStorage.setItem(${JSON.stringify(OVERRIDES_KEY)}, serialized)` : ''} return overrides `)) as Record } -export function writeOverride(key: string, override: FlagOverride): Promise> { +export function writeOverride(key: string, override: FlagOverride, site?: string): Promise> { // Parse the override from JSON *data* rather than interpolating an object literal, so a value // property named "__proto__" stays real data instead of the prototype setter (which would silently // drop it and persist {}). const overrideJson = JSON.stringify(JSON.stringify(override)) - return applyOverrideStatement(`overrides[${JSON.stringify(key)}] = JSON.parse(${overrideJson})`) + return applyOverrideStatement(`overrides[${JSON.stringify(key)}] = JSON.parse(${overrideJson})`, site) } -export function deleteOverride(key: string): Promise> { - return applyOverrideStatement(`delete overrides[${JSON.stringify(key)}]`) +export function deleteOverride(key: string, site?: string): Promise> { + return applyOverrideStatement(`delete overrides[${JSON.stringify(key)}]`, site) } -export function clearAllOverrides(): Promise> { - return applyOverrideStatement('overrides = {}') +/** + * Connected, this clears the site you're on and leaves the other sites' stores alone. Signed out + * there's no site to scope to, so it wipes every store as well as the projection — clearing only the + * projection would let the overrides reappear the moment you reconnect, which reads as the button + * not having worked. + */ +export async function clearAllOverrides(site?: string): Promise> { + if (site) { + return applyOverrideStatement('overrides = {}', site) + } + await evalInWindow(` + for (let i = localStorage.length - 1; i >= 0; i--) { + const key = localStorage.key(i) + if (key && key.indexOf(${JSON.stringify(SITE_OVERRIDES_PREFIX)}) === 0) { + localStorage.removeItem(key) + } + } + localStorage.setItem(${JSON.stringify(OVERRIDES_KEY)}, '{}') + `) + return {} } /** diff --git a/developer-extension/src/panel/components/tabs/flagsTab/useInspectedPageOverrides.ts b/developer-extension/src/panel/components/tabs/flagsTab/useInspectedPageOverrides.ts index b3b1ca7afa..aacb791d97 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/useInspectedPageOverrides.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/useInspectedPageOverrides.ts @@ -6,6 +6,7 @@ import { readFlagState, reloadInspectedPage, sanitizeOverrides, + syncSiteOverrides, writeOverride, } from './inspectedPageFlags' @@ -30,6 +31,14 @@ export interface OverridesController extends FlagPageState { clearOverride: (flagKey: string) => Promise clearAll: () => Promise reloadPage: () => void + /** + * True once scoping the page to the connected site changed which overrides apply, until a reload + * picks them up. The page is running another site's values in the meantime, so this is louder than + * the ordinary "you edited an override" nudge. + */ + siteSwitchNeedsReload: boolean + /** Set when scoping failed outright, so the page's overrides may belong to another site. */ + scopeError: string | null } function delay(ms: number): Promise { @@ -46,11 +55,12 @@ function delay(ms: number): Promise { // - no read ever succeeded -> error async function settleFlagState( setState: Dispatch>, - isCancelled: () => boolean + isCancelled: () => boolean, + site?: string ): Promise { let lastRead: FlagState | null = null for (let attempts = 1; ; attempts++) { - const next = await readFlagState() + const next = await readFlagState(site) if (isCancelled()) { return } @@ -102,14 +112,20 @@ async function settleFlagState( * before deciding it's absent. * * Assumes a single mounted instance — the mutation queue only serializes writes within one hook. + * + * `site` scopes everything to the connected Datadog site, so overrides made on one site neither + * apply nor show up on another. Omitted when signed out: there's no site to scope to then, and the + * caller wants what the page is actually applying. */ -export function useInspectedPageOverrides(): OverridesController { +export function useInspectedPageOverrides(site?: string): OverridesController { const [state, setState] = useState({ status: 'loading', overrides: {}, devtoolsEnabled: false, error: null, }) + const [siteSwitchNeedsReload, setSiteSwitchNeedsReload] = useState(false) + const [scopeError, setScopeError] = useState(null) // Serializes mutations so overlapping read-modify-writes can't clobber each other. const mutationQueue = useRef>(Promise.resolve()) // Cancels an in-flight settle when a newer navigation (or unmount) supersedes it. @@ -131,8 +147,48 @@ export function useInspectedPageOverrides(): OverridesController { cancelSettle.current = () => { cancelled = true } - void settleFlagState(setState, () => cancelled) - }, []) + void settleFlagState(setState, () => cancelled, site) + }, [site]) + + // Point the wrapper's key at this site's store whenever the connected site changes. Reruns on + // navigation too (the new page has its own localStorage), keyed off `status` returning to ready. + // + // Known limitation (accepted): this runs outside the mutation queue, which only spans one hook + // instance anyway — the provider remounts on a site change. A write dispatched just before the + // switch writes the old site's projection and can land after this sync, leaving the old site's + // overrides projected until the next sync. Each eval is atomic, so the stores stay consistent. + useEffect(() => { + if (!site || state.status !== 'ready') { + return + } + let cancelled = false + const seq = readSeq.current + void syncSiteOverrides(site).then((result) => { + if (cancelled) { + return + } + if (!result) { + // Without a successful projection the page may still be applying another site's overrides, + // and the list below wouldn't show it. Never fail this silently. + setScopeError("Couldn't scope overrides to this site. The page may still be applying another site's.") + return + } + setScopeError(null) + // Only a projection that actually changed leaves the page running the wrong values; an + // unchanged one must stay silent, or every sign-in would demand a pointless reload. + if (result.changed) { + setSiteSwitchNeedsReload(true) + } + // Adoption can put overrides in the store after the settle read found none. Skipped if a write + // or navigation landed meanwhile, since that result is newer. + if (seq === readSeq.current) { + setState((prev) => ({ ...prev, overrides: result.overrides })) + } + }) + return () => { + cancelled = true + } + }, [site, state.status]) // Known limitation (accepted): terminal events aren't correlated to a specific navigation — the // webNavigation API exposes no id spanning onBeforeNavigate→onCompleted. In a rare overlapping- @@ -154,6 +210,9 @@ export function useInspectedPageOverrides(): OverridesController { // (before the re-render mirrors statusRef from state). readSeq.current += 1 statusRef.current = 'loading' + // The reload the banner asked for may be this navigation, however it was triggered. The sync + // that runs once the new page settles raises it again if it's still needed. + setSiteSwitchNeedsReload(false) setState((prev) => ({ ...prev, status: 'loading', error: null })) } const onNavigationSettled = (details: { tabId: number; frameId: number }) => { @@ -201,13 +260,13 @@ export function useInspectedPageOverrides(): OverridesController { }, []) const setOverride = useCallback( - (flagKey: string, override: FlagOverride) => enqueue(() => writeOverride(flagKey, override)), - [enqueue] + (flagKey: string, override: FlagOverride) => enqueue(() => writeOverride(flagKey, override, site)), + [enqueue, site] ) - const clearOverride = useCallback((flagKey: string) => enqueue(() => deleteOverride(flagKey)), [enqueue]) + const clearOverride = useCallback((flagKey: string) => enqueue(() => deleteOverride(flagKey, site)), [enqueue, site]) - const clearAll = useCallback(() => enqueue(() => clearAllOverrides()), [enqueue]) + const clearAll = useCallback(() => enqueue(() => clearAllOverrides(site)), [enqueue, site]) const reloadPage = useCallback(() => reloadInspectedPage(), []) @@ -220,6 +279,8 @@ export function useInspectedPageOverrides(): OverridesController { clearOverride, clearAll, reloadPage, + siteSwitchNeedsReload, + scopeError, } } diff --git a/developer-extension/src/panel/components/tabs/flagsTab/useOverriddenFlags.ts b/developer-extension/src/panel/components/tabs/flagsTab/useOverriddenFlags.ts index 3354dbd51a..b80e3048a3 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/useOverriddenFlags.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/useOverriddenFlags.ts @@ -19,8 +19,8 @@ const EMPTY: OverriddenFlagsState = { flags: [], missingKeys: new Set() } * Loads the catalog data (name, variants, type) for the currently-overridden flag keys, so the * "Local overrides" section can render them even when they're not on the current catalog page. A key * that doesn't resolve falls back to a minimal row so the override can still be reverted, and lands - * in `missingKeys` — which the row turns into a "clear this" warning, so only proven absence counts. - * Failures are non-blocking: the section shows fallback rows, unmarked. + * in `missingKeys`, which the row notes as archived or deleted. Failures are non-blocking: the + * section shows fallback rows, unmarked. */ export function useOverriddenFlags(auth: FlagAuthState, keys: string[]): OverriddenFlagsState { const { isConnected, site } = auth From 8754fd4cefa128ab8bc679da2580667e478d67d4 Mon Sep 17 00:00:00 2001 From: Kelly Date: Thu, 20 Aug 2026 03:53:09 -0400 Subject: [PATCH 04/12] =?UTF-8?q?=F0=9F=93=9D=20Apply=20review=20wording?= =?UTF-8?q?=20nits=20on=20the=20signed-out=20overrides=20notice?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5 (1M context) --- .../src/panel/components/tabs/flagsTab/connectScreen.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx b/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx index d5e604beab..0c23c420e6 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx +++ b/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx @@ -43,7 +43,7 @@ export function ConnectScreen({ auth }: { auth: FlagAuthState }) { /** * Surfaces overrides already stored on the inspected page while signed out, with a Clear all so they - * can be wiped without signing in just for that. + * can be wiped without signing in. * * Mounted only while disconnected, so its navigation listeners never run alongside the connected * tab's own instance of this hook. @@ -91,7 +91,7 @@ function DisconnectedOverridesNotice() { return ( - Reload the page to stop applying them. + Please reload the page to stop applying them. From 544faffd050d42af29f9f8933d369f5edfcdc3ff Mon Sep 17 00:00:00 2001 From: Kelly Date: Thu, 20 Aug 2026 04:11:19 -0400 Subject: [PATCH 05/12] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Share=20one=20storag?= =?UTF-8?q?e=20parser=20across=20the=20flag=20override=20evals,=20tighten?= =?UTF-8?q?=20comments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sync path had grown its own JSON parsing while the read and write paths used another, which is what the shared prelude existed to prevent. Both now use one `parse`/`stable` helper block. Comments trimmed to what isn't already obvious from the code. Co-Authored-By: Claude Opus 5 (1M context) --- .../tabs/flagsTab/connectScreen.tsx | 4 +- .../tabs/flagsTab/flagCatalogList.tsx | 6 +- .../components/tabs/flagsTab/flagsTab.tsx | 4 +- .../tabs/flagsTab/inspectedPageFlags.ts | 88 ++++++++----------- .../flagsTab/useInspectedPageOverrides.ts | 37 +++----- 5 files changed, 57 insertions(+), 82 deletions(-) diff --git a/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx b/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx index 0c23c420e6..de4a2f9044 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx +++ b/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx @@ -120,8 +120,8 @@ function DisconnectedOverridesNotice() { {phase === 'confirming' || phase === 'clearing' ? ( <> - {/* Signed out there's no site to scope to, so this wipes the saved stores for every - Datadog site as well — say so rather than deleting more than the count implies. */} + {/* Signed out this clears every site's overrides, not just the ones counted above, so + the prompt says so. */} Clear all overrides on this page, including any saved for other Datadog sites? diff --git a/developer-extension/src/panel/components/tabs/flagsTab/flagCatalogList.tsx b/developer-extension/src/panel/components/tabs/flagsTab/flagCatalogList.tsx index 5f38b2f31e..32c9c388a6 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/flagCatalogList.tsx +++ b/developer-extension/src/panel/components/tabs/flagsTab/flagCatalogList.tsx @@ -121,8 +121,8 @@ function FlagRow({ onRevert: (flagKey: string) => void }) { const overridden = override !== undefined - // The wrapper rejects a mismatched type at resolve time, so this override genuinely won't apply — - // unlike an unresolved key, which still resolves fine and is only worth noting. + // The wrapper rejects a mismatched type, so this override won't apply — unlike an unresolved key, + // which still resolves fine. const typeMismatch = overridden && override.type !== flag.type return ( @@ -154,7 +154,7 @@ function FlagRow({ won't apply until you clear it. )} - {/* Not an error: the override still resolves, the flag just isn't in the catalog any more. */} + {/* Not an error: the override still works, the flag just left the catalog. */} {flag.unresolved && ( No active flag with this key on this site — it may have been archived or deleted. diff --git a/developer-extension/src/panel/components/tabs/flagsTab/flagsTab.tsx b/developer-extension/src/panel/components/tabs/flagsTab/flagsTab.tsx index 19abf51d82..45b944d64f 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/flagsTab.tsx +++ b/developer-extension/src/panel/components/tabs/flagsTab/flagsTab.tsx @@ -66,8 +66,8 @@ function ConnectedFlagsTab({ auth }: { auth: FlagAuthState }) { } > - {/* Louder than the usual pending-refresh nudge: until this reload happens the page is running - a different site's overrides than the ones listed below. */} + {/* Louder than the usual refresh nudge: the page is applying a different set than the one + listed below. */} {siteSwitchNeedsReload && ( <> diff --git a/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts b/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts index 7b1d8bbca9..818574e931 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts @@ -11,13 +11,9 @@ export const OVERRIDES_KEY = 'dd.dd_flag.overrides' export const DEVTOOLS_MARKER_KEY = 'dd.dd_flag.devtools' /** - * Per-site stores, owned by this extension alone. The wrapper knows nothing about them: it only ever - * reads OVERRIDES_KEY, which we keep as a projection of the connected site's store. That's what - * stops an override made on staging from applying on US1 — the other site's copy is parked here - * rather than sitting in the key the wrapper reads. - * - * The trailing dot matters: it keeps these keys from colliding with OVERRIDES_KEY itself when we - * enumerate them. + * Per-site stores. The wrapper only reads OVERRIDES_KEY, so we keep that as a copy of the connected + * site's store and leave the other sites here, where the wrapper can't see them. The trailing dot + * stops these keys matching OVERRIDES_KEY itself. */ const SITE_OVERRIDES_PREFIX = `${OVERRIDES_KEY}.` @@ -70,26 +66,27 @@ export function sanitizeOverrides(overrides: Record): FlagOverr return sanitized } -// Shared prelude for every inspected-window eval: parses an overrides map out of localStorage, -// tolerating malformed/absent/mistyped storage, and leaves a normalized `overrides` in scope. -// Defined once so the read and mutation paths can't interpret storage differently as the contract -// evolves. `storeKey` is the site's store when connected, and OVERRIDES_KEY when signed out. -function readOverridesPrelude(storeKey: string): string { - return ` - let overrides = {} - try { - const parsed = JSON.parse(localStorage.getItem(${JSON.stringify(storeKey)}) || '{}') - if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { - overrides = parsed +// Shared by every eval so all paths read storage the same way. `parse` returns null for anything +// that isn't an overrides map; `stable` compares two maps ignoring key order. +const EVAL_HELPERS = ` + const parse = (raw) => { + try { + const parsed = JSON.parse(raw || 'null') + return parsed && typeof parsed === 'object' && !Array.isArray(parsed) ? parsed : null + } catch (e) { + return null } - } catch (e) {} + } + const stable = (map) => JSON.stringify(Object.keys(map).sort().map((k) => [k, map[k]])) ` -} -/** Key order can differ between two equal maps, so compare entries rather than raw JSON. */ -const STABLE_STRINGIFY = ` - const stable = (map) => JSON.stringify(Object.keys(map).sort().map((k) => [k, map[k]])) +// Leaves a mutable `overrides` in scope for the read and write paths. +function overridesPrelude(storeKey: string): string { + return ` + ${EVAL_HELPERS} + let overrides = parse(localStorage.getItem(${JSON.stringify(storeKey)})) || {} ` +} /** * Reads the current overrides and enablement marker straight from the inspected page's localStorage @@ -100,12 +97,12 @@ const STABLE_STRINGIFY = ` * "not detected" warning. */ export async function readFlagState(site?: string): Promise { - // Connected, so show that site's own overrides. Signed out there's no site to scope by, and what - // matters is what the page is actually applying — which is the projection. + // Signed out there's no site to scope to, so read the key the wrapper uses — that shows whatever + // the page is currently applying. const storeKey = site ? siteOverridesKey(site) : OVERRIDES_KEY try { const raw = (await evalInWindow(` - ${readOverridesPrelude(storeKey)} + ${overridesPrelude(storeKey)} const devtoolsEnabled = localStorage.getItem(${JSON.stringify(DEVTOOLS_MARKER_KEY)}) === 'enabled' return { overrides, devtoolsEnabled } `)) as FlagState @@ -117,32 +114,21 @@ export async function readFlagState(site?: string): Promise { } /** - * Points the key the wrapper reads at `site`'s store, so only that site's overrides apply. Returns - * that store, plus whether the projection actually changed: if it didn't, the page is already - * running the right values and must not be asked to reload — that's what keeps signing in inert. + * Copies `site`'s store into the key the wrapper reads, so only that site's overrides apply. + * `changed` is false when it already matched, so callers don't ask for a needless reload. * - * Also adopts pre-scoping overrides, but only while no site store holds anything. Adopting per-site - * would copy whatever is live into each site as you visit it, which is the leak this exists to stop. - * Once a store does hold something the extension owns the projection, and anything written straight - * to OVERRIDES_KEY from outside is overwritten on the next sync. + * Overrides predating this scheme are adopted by the first site to connect, and only while no store + * holds anything — adopting per-site would copy whatever is live into every site, which is the leak + * this prevents. After that, writes made straight to OVERRIDES_KEY are overwritten. * - * Returns null if the page couldn't be written to. The caller must surface that rather than assume - * success — an unprojected page keeps applying whichever site it had. + * Null means the write failed — callers must surface that, since the page then keeps applying + * whichever site's overrides it already had. */ export async function syncSiteOverrides(site: string): Promise<{ changed: boolean; overrides: FlagOverrides } | null> { const storeKey = siteOverridesKey(site) try { const result = (await evalInWindow(` - ${STABLE_STRINGIFY} - const parse = (raw) => { - try { - const parsed = JSON.parse(raw || 'null') - return parsed && typeof parsed === 'object' && !Array.isArray(parsed) ? parsed : null - } catch (e) { - return null - } - } - + ${EVAL_HELPERS} const projection = parse(localStorage.getItem(${JSON.stringify(OVERRIDES_KEY)})) || {} let siteOverrides = parse(localStorage.getItem(${JSON.stringify(storeKey)})) @@ -159,8 +145,8 @@ export async function syncSiteOverrides(site: string): Promise<{ changed: boolea } } siteOverrides = anySiteOverrides ? {} : projection - // Only persist a store with something in it. Writing an empty one on a clean page would - // disarm adoption for this origin forever, so a later hand-written override would be wiped. + // An empty store would still count as existing above and block adoption for good, so don't + // write one. if (Object.keys(siteOverrides).length > 0) { localStorage.setItem(${JSON.stringify(storeKey)}, JSON.stringify(siteOverrides)) } @@ -187,7 +173,7 @@ export async function syncSiteOverrides(site: string): Promise<{ changed: boolea async function applyOverrideStatement(statement: string, site?: string): Promise> { const storeKey = site ? siteOverridesKey(site) : OVERRIDES_KEY return (await evalInWindow(` - ${readOverridesPrelude(storeKey)} + ${overridesPrelude(storeKey)} ${statement} const serialized = JSON.stringify(overrides) localStorage.setItem(${JSON.stringify(storeKey)}, serialized) @@ -209,10 +195,8 @@ export function deleteOverride(key: string, site?: string): Promise> { if (site) { diff --git a/developer-extension/src/panel/components/tabs/flagsTab/useInspectedPageOverrides.ts b/developer-extension/src/panel/components/tabs/flagsTab/useInspectedPageOverrides.ts index aacb791d97..58cfc89fea 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/useInspectedPageOverrides.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/useInspectedPageOverrides.ts @@ -31,13 +31,9 @@ export interface OverridesController extends FlagPageState { clearOverride: (flagKey: string) => Promise clearAll: () => Promise reloadPage: () => void - /** - * True once scoping the page to the connected site changed which overrides apply, until a reload - * picks them up. The page is running another site's values in the meantime, so this is louder than - * the ordinary "you edited an override" nudge. - */ + /** Scoping changed which overrides apply; the page needs a reload to pick it up. */ siteSwitchNeedsReload: boolean - /** Set when scoping failed outright, so the page's overrides may belong to another site. */ + /** Scoping failed, so the page may be applying another site's overrides. */ scopeError: string | null } @@ -113,9 +109,8 @@ async function settleFlagState( * * Assumes a single mounted instance — the mutation queue only serializes writes within one hook. * - * `site` scopes everything to the connected Datadog site, so overrides made on one site neither - * apply nor show up on another. Omitted when signed out: there's no site to scope to then, and the - * caller wants what the page is actually applying. + * `site` scopes everything to the connected Datadog site, so overrides made on one neither apply nor + * show up on another. Omitted when signed out, where the caller wants what the page is applying. */ export function useInspectedPageOverrides(site?: string): OverridesController { const [state, setState] = useState({ @@ -150,13 +145,11 @@ export function useInspectedPageOverrides(site?: string): OverridesController { void settleFlagState(setState, () => cancelled, site) }, [site]) - // Point the wrapper's key at this site's store whenever the connected site changes. Reruns on - // navigation too (the new page has its own localStorage), keyed off `status` returning to ready. + // Scope the page to the connected site. Reruns on navigation too, via `status` returning to ready. // - // Known limitation (accepted): this runs outside the mutation queue, which only spans one hook - // instance anyway — the provider remounts on a site change. A write dispatched just before the - // switch writes the old site's projection and can land after this sync, leaving the old site's - // overrides projected until the next sync. Each eval is atomic, so the stores stay consistent. + // Known limitation (accepted): this runs outside the mutation queue, which covers one hook + // instance anyway — the provider remounts on a site change. A write sent just before the switch + // can land after this sync and restore the old site's copy until the next one. useEffect(() => { if (!site || state.status !== 'ready') { return @@ -168,19 +161,17 @@ export function useInspectedPageOverrides(site?: string): OverridesController { return } if (!result) { - // Without a successful projection the page may still be applying another site's overrides, - // and the list below wouldn't show it. Never fail this silently. + // The page may still be applying another site's overrides, and the list wouldn't show it. setScopeError("Couldn't scope overrides to this site. The page may still be applying another site's.") return } setScopeError(null) - // Only a projection that actually changed leaves the page running the wrong values; an - // unchanged one must stay silent, or every sign-in would demand a pointless reload. + // Prompting when nothing changed would make every sign-in demand a needless reload. if (result.changed) { setSiteSwitchNeedsReload(true) } - // Adoption can put overrides in the store after the settle read found none. Skipped if a write - // or navigation landed meanwhile, since that result is newer. + // Adoption can fill the store after the settle read found none. Skipped if a newer write or + // navigation already landed. if (seq === readSeq.current) { setState((prev) => ({ ...prev, overrides: result.overrides })) } @@ -210,8 +201,8 @@ export function useInspectedPageOverrides(site?: string): OverridesController { // (before the re-render mirrors statusRef from state). readSeq.current += 1 statusRef.current = 'loading' - // The reload the banner asked for may be this navigation, however it was triggered. The sync - // that runs once the new page settles raises it again if it's still needed. + // This may be the reload the banner asked for. If the page still needs one, the sync that + // runs once it settles raises the banner again. setSiteSwitchNeedsReload(false) setState((prev) => ({ ...prev, status: 'loading', error: null })) } From e59481bbd2d696c13e2c204f65305b14062fe9ce Mon Sep 17 00:00:00 2001 From: Kelly Date: Thu, 20 Aug 2026 04:18:47 -0400 Subject: [PATCH 06/12] =?UTF-8?q?=F0=9F=93=9D=20Drop=20flag=20override=20c?= =?UTF-8?q?omments=20the=20code=20already=20states?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5 (1M context) --- .../panel/components/tabs/flagsTab/connectScreen.tsx | 2 -- .../panel/components/tabs/flagsTab/flagCatalogList.tsx | 3 +-- .../src/panel/components/tabs/flagsTab/flagsTab.tsx | 2 -- .../components/tabs/flagsTab/inspectedPageFlags.ts | 10 ++++------ .../tabs/flagsTab/useInspectedPageOverrides.ts | 6 ++---- 5 files changed, 7 insertions(+), 16 deletions(-) diff --git a/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx b/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx index de4a2f9044..87be0770a0 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx +++ b/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx @@ -120,8 +120,6 @@ function DisconnectedOverridesNotice() { {phase === 'confirming' || phase === 'clearing' ? ( <> - {/* Signed out this clears every site's overrides, not just the ones counted above, so - the prompt says so. */} Clear all overrides on this page, including any saved for other Datadog sites? diff --git a/developer-extension/src/panel/components/tabs/flagsTab/flagCatalogList.tsx b/developer-extension/src/panel/components/tabs/flagsTab/flagCatalogList.tsx index 32c9c388a6..b32bb40449 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/flagCatalogList.tsx +++ b/developer-extension/src/panel/components/tabs/flagsTab/flagCatalogList.tsx @@ -121,8 +121,7 @@ function FlagRow({ onRevert: (flagKey: string) => void }) { const overridden = override !== undefined - // The wrapper rejects a mismatched type, so this override won't apply — unlike an unresolved key, - // which still resolves fine. + // The wrapper rejects a mismatched type, so this override won't apply. const typeMismatch = overridden && override.type !== flag.type return ( diff --git a/developer-extension/src/panel/components/tabs/flagsTab/flagsTab.tsx b/developer-extension/src/panel/components/tabs/flagsTab/flagsTab.tsx index 45b944d64f..b577d13973 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/flagsTab.tsx +++ b/developer-extension/src/panel/components/tabs/flagsTab/flagsTab.tsx @@ -66,8 +66,6 @@ function ConnectedFlagsTab({ auth }: { auth: FlagAuthState }) { } > - {/* Louder than the usual refresh nudge: the page is applying a different set than the one - listed below. */} {siteSwitchNeedsReload && ( <> diff --git a/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts b/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts index 818574e931..8e7dfee805 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts @@ -66,8 +66,8 @@ export function sanitizeOverrides(overrides: Record): FlagOverr return sanitized } -// Shared by every eval so all paths read storage the same way. `parse` returns null for anything -// that isn't an overrides map; `stable` compares two maps ignoring key order. +// Shared by every eval so all paths read storage the same way. `stable` ignores key order, so two +// equal maps compare equal. const EVAL_HELPERS = ` const parse = (raw) => { try { @@ -97,8 +97,7 @@ function overridesPrelude(storeKey: string): string { * "not detected" warning. */ export async function readFlagState(site?: string): Promise { - // Signed out there's no site to scope to, so read the key the wrapper uses — that shows whatever - // the page is currently applying. + // Signed out there's no site to scope to, so read the key the wrapper uses: what the page applies. const storeKey = site ? siteOverridesKey(site) : OVERRIDES_KEY try { const raw = (await evalInWindow(` @@ -145,8 +144,7 @@ export async function syncSiteOverrides(site: string): Promise<{ changed: boolea } } siteOverrides = anySiteOverrides ? {} : projection - // An empty store would still count as existing above and block adoption for good, so don't - // write one. + // An empty store would still count as existing above, blocking adoption for good. if (Object.keys(siteOverrides).length > 0) { localStorage.setItem(${JSON.stringify(storeKey)}, JSON.stringify(siteOverrides)) } diff --git a/developer-extension/src/panel/components/tabs/flagsTab/useInspectedPageOverrides.ts b/developer-extension/src/panel/components/tabs/flagsTab/useInspectedPageOverrides.ts index 58cfc89fea..f8df379590 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/useInspectedPageOverrides.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/useInspectedPageOverrides.ts @@ -161,7 +161,6 @@ export function useInspectedPageOverrides(site?: string): OverridesController { return } if (!result) { - // The page may still be applying another site's overrides, and the list wouldn't show it. setScopeError("Couldn't scope overrides to this site. The page may still be applying another site's.") return } @@ -171,7 +170,7 @@ export function useInspectedPageOverrides(site?: string): OverridesController { setSiteSwitchNeedsReload(true) } // Adoption can fill the store after the settle read found none. Skipped if a newer write or - // navigation already landed. + // navigation landed meanwhile. if (seq === readSeq.current) { setState((prev) => ({ ...prev, overrides: result.overrides })) } @@ -201,8 +200,7 @@ export function useInspectedPageOverrides(site?: string): OverridesController { // (before the re-render mirrors statusRef from state). readSeq.current += 1 statusRef.current = 'loading' - // This may be the reload the banner asked for. If the page still needs one, the sync that - // runs once it settles raises the banner again. + // This may be the reload the banner asked for; the sync after settling re-raises it if not. setSiteSwitchNeedsReload(false) setState((prev) => ({ ...prev, status: 'loading', error: null })) } From 51c69a7117bc7e2236be10eba2116c4214fe297a Mon Sep 17 00:00:00 2001 From: Kelly Date: Thu, 20 Aug 2026 04:23:20 -0400 Subject: [PATCH 07/12] =?UTF-8?q?=F0=9F=90=9B=20Show=20stored=20overrides?= =?UTF-8?q?=20from=20every=20site=20on=20the=20signed-out=20notice?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A site switch repoints the projection before the reload that picks it up, so the page can still be applying the site it loaded with. The signed-out notice read only the projection, so after switching without reloading it showed nothing — no warning and no Clear all — while an override was still in effect. It now reports every stored override, which is also what Clear all wipes there. Copy follows: stored for this page, may still be applying. Co-Authored-By: Claude Opus 5 (1M context) --- .../tabs/flagsTab/connectScreen.tsx | 4 ++-- .../tabs/flagsTab/inspectedPageFlags.spec.ts | 13 ++++++++--- .../tabs/flagsTab/inspectedPageFlags.ts | 23 ++++++++++++++++--- 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx b/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx index 87be0770a0..d64f1b2516 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx +++ b/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx @@ -111,10 +111,10 @@ function DisconnectedOverridesNotice() { color="orange" w="100%" data-dd-privacy="mask" - title={`${count} override${count === 1 ? '' : 's'} active on this page`} + title={`${count} override${count === 1 ? '' : 's'} stored for this page`} > - These are stored in the page and keep applying while you are signed out. Sign in to review and remove them + These stay in the page and may still be applying while you are signed out. Sign in to review and remove them individually. diff --git a/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.spec.ts b/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.spec.ts index 2d8c1147fb..afe102659a 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.spec.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.spec.ts @@ -140,14 +140,21 @@ describe('inspectedPageFlags read/write against page localStorage', () => { expect(stored(OVERRIDES_KEY)).toEqual({ 'dark-mode': override }) }) - it('reads the connected site when given one, and what the page is applying when not', async () => { + it('reads only the connected site when given one', async () => { await writeOverride('dark-mode', override, STAGING) await syncSiteOverrides(US1) - // US1 has none of its own; signed out we report the projection, which is now US1's (empty). expect((await readFlagState(STAGING))?.overrides).toEqual({ 'dark-mode': override }) expect((await readFlagState(US1))?.overrides).toEqual({}) - expect((await readFlagState())?.overrides).toEqual({}) + }) + + it('reports every stored override when signed out, even one the page has not reloaded away from', async () => { + await writeOverride('dark-mode', override, STAGING) + // Switching empties the projection, but the page keeps applying staging's until it reloads — + // so the signed-out notice must still offer to clear it. + await syncSiteOverrides(US1) + + expect((await readFlagState())?.overrides).toEqual({ 'dark-mode': override }) }) it("stops one site's override from applying on another", async () => { diff --git a/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts b/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts index 8e7dfee805..eb40544565 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts @@ -97,11 +97,28 @@ function overridesPrelude(storeKey: string): string { * "not detected" warning. */ export async function readFlagState(site?: string): Promise { - // Signed out there's no site to scope to, so read the key the wrapper uses: what the page applies. - const storeKey = site ? siteOverridesKey(site) : OVERRIDES_KEY + // Signed out we can't tell what the page is applying: a site switch repoints the projection before + // the reload that picks it up, so the page can still be running the site it loaded with. Report + // everything stored instead — which is also exactly what Clear all wipes there. + const source = site + ? overridesPrelude(siteOverridesKey(site)) + : ` + ${overridesPrelude(OVERRIDES_KEY)} + for (let i = 0; i < localStorage.length; i++) { + const key = localStorage.key(i) + if (key && key.indexOf(${JSON.stringify(SITE_OVERRIDES_PREFIX)}) === 0) { + const store = parse(localStorage.getItem(key)) || {} + for (const flagKey of Object.keys(store)) { + if (!Object.prototype.hasOwnProperty.call(overrides, flagKey)) { + overrides[flagKey] = store[flagKey] + } + } + } + } + ` try { const raw = (await evalInWindow(` - ${overridesPrelude(storeKey)} + ${source} const devtoolsEnabled = localStorage.getItem(${JSON.stringify(DEVTOOLS_MARKER_KEY)}) === 'enabled' return { overrides, devtoolsEnabled } `)) as FlagState From f6ee684022726cd011054db89758e5952ac161ba Mon Sep 17 00:00:00 2001 From: Kelly Date: Thu, 20 Aug 2026 11:25:24 -0400 Subject: [PATCH 08/12] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Drop=20the=20pre-sco?= =?UTF-8?q?ping=20override=20migration,=20clarify=20the=20reload=20banner?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tab shipped days ago, so there's no override history worth carrying forward. Overrides left in the wrapper key by an older build are now simply dropped on first connect instead of being adopted into the first site. That removes the adoption gate and, with it, the reason syncSiteOverrides returned the store at all — so the hook's sequence guard and its extra state write go too. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/tabs/flagsTab/flagsTab.tsx | 2 +- .../tabs/flagsTab/inspectedPageFlags.spec.ts | 26 +------------ .../tabs/flagsTab/inspectedPageFlags.ts | 37 ++++--------------- .../flagsTab/useInspectedPageOverrides.ts | 10 +---- 4 files changed, 12 insertions(+), 63 deletions(-) diff --git a/developer-extension/src/panel/components/tabs/flagsTab/flagsTab.tsx b/developer-extension/src/panel/components/tabs/flagsTab/flagsTab.tsx index b577d13973..31dc7e7684 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/flagsTab.tsx +++ b/developer-extension/src/panel/components/tabs/flagsTab/flagsTab.tsx @@ -71,7 +71,7 @@ function ConnectedFlagsTab({ auth }: { auth: FlagAuthState }) { - Which overrides apply changed when you switched sites, and the page hasn't reloaded since. + You switched sites, so a different set of overrides applies now. Reload the page to pick them up.