From 2acd085729d5d6f3c644fcfd13412dd2b95e48f8 Mon Sep 17 00:00:00 2001 From: Arjun Mahanti Date: Wed, 26 Aug 2026 19:34:37 -0400 Subject: [PATCH] fix(desktop): fail closed for unavailable previews Co-authored-by: Codex Signed-off-by: Arjun Mahanti --- .../src/features/settings/ui/SettingsView.tsx | 11 +++-- .../features/buildAvailability.test.mjs | 7 ++++ desktop/src/shared/features/index.ts | 6 ++- .../shared/features/resolveEnabled.test.mjs | 42 +++++++++++++++---- desktop/src/shared/features/resolveEnabled.ts | 21 ++++++---- .../features/runtimeBuildAvailability.ts | 14 +++++++ .../src/shared/features/useFeatureEnabled.ts | 12 +----- 7 files changed, 82 insertions(+), 31 deletions(-) diff --git a/desktop/src/features/settings/ui/SettingsView.tsx b/desktop/src/features/settings/ui/SettingsView.tsx index 2f9b2c36a1..b3bcaf7586 100644 --- a/desktop/src/features/settings/ui/SettingsView.tsx +++ b/desktop/src/features/settings/ui/SettingsView.tsx @@ -7,11 +7,11 @@ import { canManageCommunityMembers, shouldWarnMissingMembershipSnapshot, } from "@/shared/api/relayMembers"; -import { getFeature } from "@/shared/features/manifest"; import { - resolveEnabled, + getFeature, + resolveFeatureEnabledInThisBuild, useFeatureSnapshot, -} from "@/shared/features/useFeatureEnabled"; +} from "@/shared/features"; import { topChromeBackdrop } from "@/shared/layout/chromeLayout"; import { cn } from "@/shared/lib/cn"; import { @@ -137,7 +137,10 @@ export function SettingsView({ // stable and renders unconditionally (fail-open). if (s.featureGate) { const feature = getFeature(s.featureGate); - if (feature && !resolveEnabled(s.featureGate, featureState)) { + if ( + feature && + !resolveFeatureEnabledInThisBuild(feature, featureState) + ) { return false; } } diff --git a/desktop/src/shared/features/buildAvailability.test.mjs b/desktop/src/shared/features/buildAvailability.test.mjs index 046615876c..688afe59bf 100644 --- a/desktop/src/shared/features/buildAvailability.test.mjs +++ b/desktop/src/shared/features/buildAvailability.test.mjs @@ -40,4 +40,11 @@ describe("feature build availability", () => { [publicFeature], ); }); + + it("includes the Bestie experiment only in an eligible build", () => { + assert.deepEqual( + filterBuildAvailableFeatures([publicFeature, bestie], { bestie: true }), + [publicFeature, bestie], + ); + }); }); diff --git a/desktop/src/shared/features/index.ts b/desktop/src/shared/features/index.ts index f476cbd14d..e53c1d1ad4 100644 --- a/desktop/src/shared/features/index.ts +++ b/desktop/src/shared/features/index.ts @@ -1,6 +1,9 @@ export { FeatureGate } from "./FeatureGate"; export { allFeatures, desktopFeatures, getFeature, manifest } from "./manifest"; -export { availableDesktopFeatures } from "./runtimeBuildAvailability"; +export { + availableDesktopFeatures, + resolveFeatureEnabledInThisBuild, +} from "./runtimeBuildAvailability"; export { getOverrides, setOverride, clearOverride } from "./store"; export type { FeatureDefinition, @@ -13,5 +16,4 @@ export { useFeatureToggle, useFeatureSnapshot, usePreviewFeatureWarning, - resolveEnabled, } from "./useFeatureEnabled"; diff --git a/desktop/src/shared/features/resolveEnabled.test.mjs b/desktop/src/shared/features/resolveEnabled.test.mjs index dc88aa8db7..bf08878a78 100644 --- a/desktop/src/shared/features/resolveEnabled.test.mjs +++ b/desktop/src/shared/features/resolveEnabled.test.mjs @@ -4,31 +4,48 @@ import { describe, it } from "node:test"; import { resolveEnabled } from "./resolveEnabled.ts"; describe("resolveEnabled (preview-only)", () => { + const workflows = { + id: "workflows", + name: "Workflows", + description: "", + }; + it("returns false by default (no override)", () => { - assert.equal(resolveEnabled("workflows", {}), false); + assert.equal(resolveEnabled(workflows, {}, true), false); }); it("returns true when user opts in", () => { - assert.equal(resolveEnabled("workflows", { workflows: true }), true); + assert.equal(resolveEnabled(workflows, { workflows: true }, true), true); }); it("uses an enabled manifest default when no override exists", () => { - assert.equal(resolveEnabled("defaultOnFeature", {}, true), true); + assert.equal( + resolveEnabled( + { ...workflows, id: "defaultOnFeature", defaultEnabled: true }, + {}, + true, + ), + true, + ); }); it("lets an explicit opt-out override an enabled default", () => { assert.equal( - resolveEnabled("defaultOnFeature", { defaultOnFeature: false }, true), + resolveEnabled( + { ...workflows, id: "defaultOnFeature", defaultEnabled: true }, + { defaultOnFeature: false }, + true, + ), false, ); }); it("returns false when user explicitly opts out", () => { - assert.equal(resolveEnabled("workflows", { workflows: false }), false); + assert.equal(resolveEnabled(workflows, { workflows: false }, true), false); }); it("ignores overrides for unrelated ids", () => { - assert.equal(resolveEnabled("workflows", { pulse: true }), false); + assert.equal(resolveEnabled(workflows, { pulse: true }, true), false); }); it("requires both build availability and a user opt-in", () => { @@ -41,9 +58,20 @@ describe("resolveEnabled (preview-only)", () => { for (const { buildAvailable, optedIn, expected } of cases) { assert.equal( - resolveEnabled("bestie", { bestie: optedIn }, false, buildAvailable), + resolveEnabled( + { ...workflows, id: "bestie" }, + { bestie: optedIn }, + buildAvailable, + ), expected, ); } }); + + it("fails closed when a JavaScript caller omits build availability", () => { + assert.equal( + resolveEnabled({ ...workflows, id: "bestie" }, { bestie: true }), + false, + ); + }); }); diff --git a/desktop/src/shared/features/resolveEnabled.ts b/desktop/src/shared/features/resolveEnabled.ts index 3aee2738dd..e89b06544e 100644 --- a/desktop/src/shared/features/resolveEnabled.ts +++ b/desktop/src/shared/features/resolveEnabled.ts @@ -1,21 +1,26 @@ /** * Pure resolution logic for preview-feature visibility. - * No side effects, no imports — safe to test in isolation. + * No side effects — safe to test in isolation. * * The manifest (`preview-features.json`) lists only preview features. * Anything not in the manifest is stable and resolves true elsewhere * (see `useFeatureEnabled`). Once you're inside `resolveEnabled`, the * feature IS in the manifest — preview by definition. * - * An explicit user override wins; otherwise the feature's manifest default is - * used (false when omitted). + * Build availability is mandatory. Within an eligible build, an explicit user + * override wins; otherwise the feature's manifest default is used (false when + * omitted). */ +import type { FeatureDefinition } from "./types"; + export function resolveEnabled( - featureId: string, + feature: FeatureDefinition, overrides: Record, - defaultEnabled = false, - buildAvailable = true, + buildAvailable: boolean, ): boolean { - if (!buildAvailable) return false; - return overrides[featureId] ?? defaultEnabled; + // Strict equality is intentional: JavaScript callers that omit the + // capability argument fail closed instead of recreating a build-gate side + // door outside TypeScript. + if (buildAvailable !== true) return false; + return overrides[feature.id] ?? feature.defaultEnabled ?? false; } diff --git a/desktop/src/shared/features/runtimeBuildAvailability.ts b/desktop/src/shared/features/runtimeBuildAvailability.ts index d0966f7095..76a8689115 100644 --- a/desktop/src/shared/features/runtimeBuildAvailability.ts +++ b/desktop/src/shared/features/runtimeBuildAvailability.ts @@ -4,6 +4,8 @@ import { type FeatureBuildFlags, } from "./buildAvailability"; import { desktopFeatures } from "./manifest"; +import { resolveEnabled } from "./resolveEnabled"; +import type { FeatureDefinition } from "./types"; const runtimeBuildFlags: FeatureBuildFlags = { bestie: import.meta.env.VITE_BUZZ_BESTIE === "1", @@ -15,6 +17,18 @@ export function isFeatureAvailableInThisBuild( return isFeatureBuildAvailable(feature, runtimeBuildFlags); } +/** Canonical preview resolution for the running build. */ +export function resolveFeatureEnabledInThisBuild( + feature: FeatureDefinition, + overrides: Record, +): boolean { + return resolveEnabled( + feature, + overrides, + isFeatureAvailableInThisBuild(feature), + ); +} + export const availableDesktopFeatures = filterBuildAvailableFeatures( desktopFeatures, runtimeBuildFlags, diff --git a/desktop/src/shared/features/useFeatureEnabled.ts b/desktop/src/shared/features/useFeatureEnabled.ts index 356a362cca..0326fe51db 100644 --- a/desktop/src/shared/features/useFeatureEnabled.ts +++ b/desktop/src/shared/features/useFeatureEnabled.ts @@ -1,7 +1,6 @@ import { useSyncExternalStore, useCallback, useEffect } from "react"; import { getFeature } from "./manifest"; -import { resolveEnabled } from "./resolveEnabled"; -import { isFeatureAvailableInThisBuild } from "./runtimeBuildAvailability"; +import { resolveFeatureEnabledInThisBuild } from "./runtimeBuildAvailability"; import { getOverrides, setOverride, OVERRIDES_KEY } from "./store"; type Listener = () => void; @@ -103,12 +102,7 @@ export function useFeatureEnabled(featureId: string): boolean { return true; } - return resolveEnabled( - featureId, - overrides, - feature.defaultEnabled, - isFeatureAvailableInThisBuild(feature), - ); + return resolveFeatureEnabledInThisBuild(feature, overrides); } /** @@ -163,5 +157,3 @@ export function usePreviewFeatureWarning(featureId: string): void { }; }, [feature, enabled]); } - -export { resolveEnabled } from "./resolveEnabled";