fix: address review feedback — migration, reactivity, standards, tests

1. Migration: existing users auto-seed experimental features as enabled
   on first load (no feature loss on upgrade). New installs get opt-in UX.
2. Reactivity: SettingsView uses useFeatureSnapshot() so section list
   updates instantly when toggles change (fixes stale useMemo deps).
3. Standards: Switch component replaces raw checkboxes, data-testid on
   every toggle (feature-toggle-{id}, feature-toggle-dev-global).
4. Tests: 11 unit tests for resolveEnabled covering all tiers + edge cases.
5. Architecture: @features-manifest Vite alias replaces fragile ../../../../
   relative import. Test loader also resolves the alias.
6. Versioned localStorage: keys now use -v1 suffix aligned to manifest version.
7. Cache: JSON.parse result cached across hook instances per render cycle.
8. Dev warning: console.warn in dev when getFeature() returns undefined.

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>

desktop/src/features-manifest.d.ts

@@ -0,0 +1,4 @@
+declare module "@features-manifest" {
+  const manifest: import("@/shared/features/types").FeaturesManifest;
+  export default manifest;
+}

desktop/src/features/settings/ui/ExperimentalFeaturesCard.tsx

@@ -1,5 +1,6 @@
 import { desktopFeatures, useFeatureToggle, useDevToggle } from "@/shared/features";
 import type { FeatureDefinition } from "@/shared/features";
+import { Switch } from "@/shared/ui/switch";
 
 function FeatureRow({ feature }: { feature: FeatureDefinition }) {
   const [enabled, toggle] = useFeatureToggle(feature.id);
@@ -10,11 +11,10 @@ function FeatureRow({ feature }: { feature: FeatureDefinition }) {
         <p className="text-sm font-medium">{feature.name}</p>
         <p className="text-xs text-muted-foreground">{feature.description}</p>
       </div>
-      <input
+      <Switch
         checked={enabled}
-        className="h-4 w-4 accent-primary"
-        onChange={(e) => toggle(e.target.checked)}
-        type="checkbox"
+        data-testid={`feature-toggle-${feature.id}`}
+        onCheckedChange={toggle}
       />
     </label>
   );
@@ -66,11 +66,10 @@ export function ExperimentalFeaturesCard() {
                 When off, all dev-tier features are hidden
               </p>
             </div>
-            <input
+            <Switch
               checked={devEnabled}
-              className="h-4 w-4 accent-primary"
-              onChange={(e) => setDevEnabled(e.target.checked)}
-              type="checkbox"
+              data-testid="feature-toggle-dev-global"
+              onCheckedChange={setDevEnabled}
             />
           </label>
 

desktop/src/features/settings/ui/SettingsView.tsx

@@ -4,8 +4,10 @@ import { ArrowLeft } from "lucide-react";
 
 import { useMyRelayMembershipQuery } from "@/features/relay-members/hooks";
 import { getFeature } from "@/shared/features/manifest";
-import { getOverrides, getDevToggle } from "@/shared/features/store";
-import { resolveEnabled } from "@/shared/features/useFeatureEnabled";
+import {
+  resolveEnabled,
+  useFeatureSnapshot,
+} from "@/shared/features/useFeatureEnabled";
 import { cn } from "@/shared/lib/cn";
 import {
   Sidebar,
@@ -117,16 +119,15 @@ export function SettingsView({
 }: SettingsViewProps) {
   const { isMobile, open: sidebarOpen, setOpen: setSidebarOpen } = useSidebar();
   const myMembershipQuery = useMyRelayMembershipQuery();
+  const featureState = useFeatureSnapshot();
   const visibleSections = React.useMemo(() => {
     const membership = myMembershipQuery.data;
-    const overrides = getOverrides();
-    const devToggle = getDevToggle();
 
     return settingsSections.filter((s) => {
       // Feature gate check
       if (s.featureGate) {
         const feature = getFeature(s.featureGate);
-        if (feature && !resolveEnabled(feature.tier, feature.id, overrides, devToggle)) {
+        if (feature && !resolveEnabled(feature.tier, feature.id, featureState.o, featureState.d)) {
           return false;
         }
       }
@@ -139,7 +140,7 @@ export function SettingsView({
       }
       return true;
     });
-  }, [myMembershipQuery.data]);
+  }, [myMembershipQuery.data, featureState]);
 
   const [isLoaded, setIsLoaded] = React.useState(false);
   const [appVersion, setAppVersion] = React.useState<string | null>(null);

desktop/src/main.tsx

@@ -2,6 +2,7 @@ import React from "react";
 import ReactDOM from "react-dom/client";
 import { App } from "@/app/App";
 import "@/shared/styles/globals.css";
+import { runMigrationIfNeeded } from "@/shared/features";
 import { UpdaterProvider } from "@/features/settings/hooks/UpdaterProvider";
 import { WorkspacesProvider } from "@/features/workspaces/useWorkspaces";
 import { ThemeProvider } from "@/shared/theme/ThemeProvider";
@@ -45,6 +46,7 @@ async function installE2eBridgeIfConfigured() {
 
 async function bootstrap() {
   await installE2eBridgeIfConfigured();
+  runMigrationIfNeeded();
   renderApp();
 }
 

desktop/src/shared/features/index.ts

@@ -1,6 +1,11 @@
 export { FeatureGate } from "./FeatureGate";
 export { allFeatures, desktopFeatures, getFeature } from "./manifest";
-export { getOverrides, setOverride, clearOverride } from "./store";
+export {
+  getOverrides,
+  setOverride,
+  clearOverride,
+  runMigrationIfNeeded,
+} from "./store";
 export type {
   FeatureDefinition,
   FeaturesManifest,
@@ -11,5 +16,6 @@ export {
   useFeatureEnabled,
   useFeatureToggle,
   useDevToggle,
+  useFeatureSnapshot,
   resolveEnabled,
 } from "./useFeatureEnabled";

desktop/src/shared/features/manifest.ts

@@ -1,4 +1,4 @@
-import manifestJson from "../../../../features.json";
+import manifestJson from "@features-manifest";
 import type { FeatureDefinition, FeaturesManifest } from "./types";
 
 const manifest = manifestJson as FeaturesManifest;

desktop/src/shared/features/resolveEnabled.test.mjs

@@ -0,0 +1,76 @@
+import assert from "node:assert/strict";
+import { describe, it } from "node:test";
+
+import { resolveEnabled } from "./resolveEnabled.ts";
+
+describe("resolveEnabled", () => {
+  describe("stable tier", () => {
+    it("always returns true regardless of overrides or env", () => {
+      assert.equal(resolveEnabled("stable", "channels", {}, false, false), true);
+      assert.equal(resolveEnabled("stable", "channels", { channels: false }, false, true), true);
+      assert.equal(resolveEnabled("stable", "channels", {}, true, true), true);
+    });
+  });
+
+  describe("experimental tier", () => {
+    it("returns false by default (no override)", () => {
+      assert.equal(resolveEnabled("experimental", "workflows", {}, true, true), false);
+    });
+
+    it("returns true when user opts in", () => {
+      assert.equal(
+        resolveEnabled("experimental", "workflows", { workflows: true }, true, true),
+        true,
+      );
+    });
+
+    it("returns false when user explicitly opts out", () => {
+      assert.equal(
+        resolveEnabled("experimental", "workflows", { workflows: false }, true, true),
+        false,
+      );
+    });
+
+    it("ignores dev toggle and isDev", () => {
+      assert.equal(
+        resolveEnabled("experimental", "workflows", { workflows: true }, false, false),
+        true,
+      );
+    });
+  });
+
+  describe("dev tier", () => {
+    it("returns false in production builds even with devToggle on", () => {
+      assert.equal(resolveEnabled("dev", "doctor", {}, true, false), false);
+    });
+
+    it("returns false in dev builds when devToggle is off", () => {
+      assert.equal(resolveEnabled("dev", "doctor", {}, false, true), false);
+    });
+
+    it("returns true in dev builds with devToggle on and no override", () => {
+      assert.equal(resolveEnabled("dev", "doctor", {}, true, true), true);
+    });
+
+    it("returns false when per-feature override is explicitly false", () => {
+      assert.equal(
+        resolveEnabled("dev", "doctor", { doctor: false }, true, true),
+        false,
+      );
+    });
+
+    it("returns true when per-feature override is explicitly true", () => {
+      assert.equal(
+        resolveEnabled("dev", "doctor", { doctor: true }, true, true),
+        true,
+      );
+    });
+  });
+
+  describe("unknown tier", () => {
+    it("returns false for unrecognized tier values", () => {
+      // @ts-expect-error — testing invalid input
+      assert.equal(resolveEnabled("unknown", "foo", {}, true, true), false);
+    });
+  });
+});

desktop/src/shared/features/resolveEnabled.ts

@@ -0,0 +1,30 @@
+import type { FeatureTier } from "./types";
+
+/**
+ * Pure resolution logic for feature visibility.
+ * No side effects, no imports beyond types — safe to test in isolation.
+ *
+ * @param isDev - Whether the current build is a dev build.
+ *               Defaults to `import.meta.env?.DEV ?? false` for runtime use.
+ */
+export function resolveEnabled(
+  tier: FeatureTier,
+  featureId: string,
+  overrides: Record<string, boolean>,
+  devToggle: boolean,
+  isDev: boolean = (import.meta.env?.DEV as boolean) ?? false,
+): boolean {
+  switch (tier) {
+    case "stable":
+      return true;
+    case "experimental":
+      return overrides[featureId] === true;
+    case "dev":
+      if (!isDev) return false;
+      if (!devToggle) return false;
+      // Allow per-feature suppression even in dev
+      return overrides[featureId] !== false;
+    default:
+      return false;
+  }
+}

desktop/src/shared/features/store.ts

@@ -1,16 +1,44 @@
 /**
  * Persistence layer for feature flag overrides.
  *
- * localStorage keys:
- *   sprout-feature-overrides  — JSON object of { [featureId]: boolean }
- *   sprout-dev-features       — "true" | "false" (global dev toggle)
+ * localStorage keys (versioned to match manifest):
+ *   sprout-feature-overrides-v1  — JSON object of { [featureId]: boolean }
+ *   sprout-dev-features-v1       — "true" | "false" (global dev toggle)
+ *   sprout-features-migrated-v1  — "true" if migration has run
  */
 
-const OVERRIDES_KEY = "sprout-feature-overrides";
-const DEV_TOGGLE_KEY = "sprout-dev-features";
+import { desktopFeatures } from "./manifest";
+
+const OVERRIDES_KEY = "sprout-feature-overrides-v1";
+const DEV_TOGGLE_KEY = "sprout-dev-features-v1";
+const MIGRATED_KEY = "sprout-features-migrated-v1";
 
 export type FeatureOverrides = Record<string, boolean>;
 
+/**
+ * One-time migration: if no overrides exist yet, seed experimental features
+ * as enabled so existing users don't lose functionality on upgrade.
+ * New installs (no prior localStorage at all) also get this — but that's fine
+ * because new users will see the features as they always have.
+ */
+export function runMigrationIfNeeded(): void {
+  try {
+    if (window.localStorage.getItem(MIGRATED_KEY) === "true") return;
+
+    // Seed all desktop experimental features as enabled
+    const seed: FeatureOverrides = {};
+    for (const f of desktopFeatures) {
+      if (f.tier === "experimental") {
+        seed[f.id] = true;
+      }
+    }
+    window.localStorage.setItem(OVERRIDES_KEY, JSON.stringify(seed));
+    window.localStorage.setItem(MIGRATED_KEY, "true");
+  } catch {
+    // localStorage unavailable — no-op
+  }
+}
+
 /** Read all user overrides from localStorage */
 export function getOverrides(): FeatureOverrides {
   try {

desktop/src/shared/features/useFeatureEnabled.ts

@@ -1,7 +1,7 @@
 import { useSyncExternalStore, useCallback } from "react";
 import { getFeature } from "./manifest";
+import { resolveEnabled } from "./resolveEnabled";
 import { getOverrides, getDevToggle, setOverride, setDevToggle } from "./store";
-import type { FeatureTier } from "./types";
 
 // ---------------------------------------------------------------------------
 // Reactive store — components re-render when overrides change
@@ -15,19 +15,55 @@ function subscribe(listener: Listener): () => void {
   return () => listeners.delete(listener);
 }
 
-function emitChange(): void {
+/** Notify all subscribers that feature state changed */
+export function emitChange(): void {
+  // Invalidate cached snapshot
+  cachedRaw = null;
+  cachedParsed = null;
   for (const listener of listeners) listener();
 }
 
-// Snapshot: a combined key of overrides + dev toggle for change detection
+// ---------------------------------------------------------------------------
+// Cached snapshot — avoids JSON.parse on every render per hook instance
+// ---------------------------------------------------------------------------
+
+interface ParsedSnapshot {
+  o: Record<string, boolean>;
+  d: boolean;
+}
+
+let cachedRaw: string | null = null;
+let cachedParsed: ParsedSnapshot | null = null;
+
 function getSnapshot(): string {
-  return JSON.stringify({ o: getOverrides(), d: getDevToggle() });
+  const raw = JSON.stringify({ o: getOverrides(), d: getDevToggle() });
+  if (raw !== cachedRaw) {
+    cachedRaw = raw;
+    cachedParsed = JSON.parse(raw) as ParsedSnapshot;
+  }
+  return raw;
+}
+
+function getParsedSnapshot(): ParsedSnapshot {
+  // Ensure snapshot is fresh
+  getSnapshot();
+  return cachedParsed!;
 }
 
 // ---------------------------------------------------------------------------
 // Public API
 // ---------------------------------------------------------------------------
 
+/**
+ * Returns the current parsed feature state (overrides + dev toggle).
+ * Reactive — re-renders when any feature toggle changes.
+ * Use this in components that need the full state (e.g. SettingsView filtering).
+ */
+export function useFeatureSnapshot(): ParsedSnapshot {
+  useSyncExternalStore(subscribe, getSnapshot, getSnapshot);
+  return getParsedSnapshot();
+}
+
 /**
  * Returns whether a feature is enabled given its tier and user overrides.
  *
@@ -36,16 +72,19 @@ function getSnapshot(): string {
  * - dev: true only if in dev build AND global dev toggle is on
  */
 export function useFeatureEnabled(featureId: string): boolean {
-  const snapshot = useSyncExternalStore(subscribe, getSnapshot, getSnapshot);
-  const parsed = JSON.parse(snapshot) as {
-    o: Record<string, boolean>;
-    d: boolean;
-  };
+  const snapshot = useFeatureSnapshot();
 
   const feature = getFeature(featureId);
-  if (!feature) return false;
+  if (!feature) {
+    if (import.meta.env.DEV) {
+      console.warn(
+        `[FeatureFlags] Unknown feature id: "${featureId}". Check features.json.`,
+      );
+    }
+    return false;
+  }
 
-  return resolveEnabled(feature.tier, featureId, parsed.o, parsed.d);
+  return resolveEnabled(feature.tier, featureId, snapshot.o, snapshot.d);
 }
 
 /**
@@ -71,38 +110,15 @@ export function useFeatureToggle(
  * Hook for the global dev toggle. Returns [enabled, toggle].
  */
 export function useDevToggle(): [boolean, (enabled: boolean) => void] {
-  const snapshot = useSyncExternalStore(subscribe, getSnapshot, getSnapshot);
-  const parsed = JSON.parse(snapshot) as { d: boolean };
+  const snapshot = useFeatureSnapshot();
 
   const toggle = useCallback((value: boolean) => {
     setDevToggle(value);
     emitChange();
   }, []);
 
-  return [parsed.d, toggle];
+  return [snapshot.d, toggle];
 }
 
-// ---------------------------------------------------------------------------
-// Pure resolution logic (exported for testing)
-// ---------------------------------------------------------------------------
-
-export function resolveEnabled(
-  tier: FeatureTier,
-  featureId: string,
-  overrides: Record<string, boolean>,
-  devToggle: boolean,
-): boolean {
-  switch (tier) {
-    case "stable":
-      return true;
-    case "experimental":
-      return overrides[featureId] === true;
-    case "dev":
-      if (!import.meta.env.DEV) return false;
-      if (!devToggle) return false;
-      // Allow per-feature suppression even in dev
-      return overrides[featureId] !== false;
-    default:
-      return false;
-  }
-}
+// Re-export for consumers that imported from here
+export { resolveEnabled } from "./resolveEnabled";

desktop/test-loader-hooks.mjs

@@ -6,7 +6,13 @@ const srcRoot = path.resolve(
   "src",
 );
 
+const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..");
+
 export function resolve(specifier, context, nextResolve) {
+  if (specifier === "@features-manifest") {
+    const resolved = path.join(repoRoot, "features.json");
+    return nextResolve(resolved, context);
+  }
   if (specifier.startsWith("@/")) {
     const resolved = `${srcRoot}/${specifier.slice(2)}.ts`;
     return nextResolve(resolved, context);

desktop/tsconfig.json

@@ -6,7 +6,8 @@
     "module": "ESNext",
     "skipLibCheck": true,
     "paths": {
-      "@/*": ["./src/*"]
+      "@/*": ["./src/*"],
+      "@features-manifest": ["../features.json"]
     },
 
     /* Bundler mode */

desktop/vite.config.ts

@@ -1,3 +1,4 @@
+import path from "node:path";
 import { defineConfig } from "vite";
 import react from "@vitejs/plugin-react";
 import { tanstackRouter } from "@tanstack/router-plugin/vite";
@@ -24,6 +25,7 @@ export default defineConfig(async () => ({
   resolve: {
     alias: {
       "@": "/src",
+      "@features-manifest": path.resolve(__dirname, "../features.json"),
     },
   },