diff --git a/Makefile b/Makefile index dd3f618c73..2379ac0120 100644 --- a/Makefile +++ b/Makefile @@ -23,6 +23,12 @@ # AVOID CONDITIONAL BRANCHES (if/else) IN BUILD TARGETS AT ALL COSTS. # Branches reduce reproducibility - builds should fail fast with clear errors # if dependencies are missing, not silently fall back to different behavior. +# +# Telemetry in Development: +# Telemetry is disabled by default in dev mode (MUX_DISABLE_TELEMETRY=1). +# To enable it (e.g., for testing PostHog experiments), set: +# MUX_ENABLE_TELEMETRY_IN_DEV=1 make dev +# This single env var is sufficient - no need to also set MUX_DISABLE_TELEMETRY=0. # Use PATH-resolved bash for portability across different systems. # - Windows: /usr/bin/bash doesn't exist in Chocolatey's make environment or GitHub Actions @@ -131,13 +137,13 @@ dev: node_modules/.installed build-main ## Start development server (Vite + node @echo "Starting dev mode (3 watchers: nodemon for main process, esbuild for api, vite for renderer)..." # On Windows, use npm run because bunx doesn't correctly pass arguments to concurrently # https://github.com/oven-sh/bun/issues/18275 - @MUX_DISABLE_TELEMETRY=$(or $(MUX_DISABLE_TELEMETRY),1) NODE_OPTIONS="--max-old-space-size=4096" npm x concurrently -k --raw \ + @MUX_DISABLE_TELEMETRY=$(if $(MUX_ENABLE_TELEMETRY_IN_DEV),,$(or $(MUX_DISABLE_TELEMETRY),1)) NODE_OPTIONS="--max-old-space-size=4096" npm x concurrently -k --raw \ "bun x nodemon --watch src --watch tsconfig.main.json --watch tsconfig.json --ext ts,tsx,json --ignore dist --ignore node_modules --exec node scripts/build-main-watch.js" \ "npx esbuild src/cli/api.ts --bundle --format=esm --platform=node --target=node20 --outfile=dist/cli/api.mjs --external:zod --external:commander --external:@trpc/server --watch" \ "vite" else dev: node_modules/.installed build-main build-preload ## Start development server (Vite + tsgo watcher for 10x faster type checking) - @MUX_DISABLE_TELEMETRY=$(or $(MUX_DISABLE_TELEMETRY),1) bun x concurrently -k \ + @MUX_DISABLE_TELEMETRY=$(if $(MUX_ENABLE_TELEMETRY_IN_DEV),,$(or $(MUX_DISABLE_TELEMETRY),1)) bun x concurrently -k \ "bun x concurrently \"$(TSGO) -w -p tsconfig.main.json\" \"bun x tsc-alias -w -p tsconfig.main.json\"" \ "bun x esbuild src/cli/api.ts --bundle --format=esm --platform=node --target=node20 --outfile=dist/cli/api.mjs --external:zod --external:commander --external:@trpc/server --watch" \ "vite" @@ -151,7 +157,7 @@ dev-server: node_modules/.installed build-main ## Start server mode with hot rel @echo "" @echo "For remote access: make dev-server VITE_HOST=0.0.0.0 BACKEND_HOST=0.0.0.0" @# On Windows, use npm run because bunx doesn't correctly pass arguments - @MUX_DISABLE_TELEMETRY=$(or $(MUX_DISABLE_TELEMETRY),1) npmx concurrently -k \ + @MUX_DISABLE_TELEMETRY=$(if $(MUX_ENABLE_TELEMETRY_IN_DEV),,$(or $(MUX_DISABLE_TELEMETRY),1)) npmx concurrently -k \ "npmx nodemon --watch src --watch tsconfig.main.json --watch tsconfig.json --ext ts,tsx,json --ignore dist --ignore node_modules --exec node scripts/build-main-watch.js" \ "npx esbuild src/cli/api.ts --bundle --format=esm --platform=node --target=node20 --outfile=dist/cli/api.mjs --external:zod --external:commander --external:@trpc/server --watch" \ "npmx nodemon --watch dist/cli/index.js --watch dist/cli/server.js --delay 500ms --exec \"node dist/cli/index.js server --host $(or $(BACKEND_HOST),localhost) --port $(or $(BACKEND_PORT),3000)\"" \ @@ -163,7 +169,7 @@ dev-server: node_modules/.installed build-main ## Start server mode with hot rel @echo " Frontend (with HMR): http://$(or $(VITE_HOST),localhost):$(or $(VITE_PORT),5173)" @echo "" @echo "For remote access: make dev-server VITE_HOST=0.0.0.0 BACKEND_HOST=0.0.0.0" - @MUX_DISABLE_TELEMETRY=$(or $(MUX_DISABLE_TELEMETRY),1) bun x concurrently -k \ + @MUX_DISABLE_TELEMETRY=$(if $(MUX_ENABLE_TELEMETRY_IN_DEV),,$(or $(MUX_DISABLE_TELEMETRY),1)) bun x concurrently -k \ "bun x concurrently \"$(TSGO) -w -p tsconfig.main.json\" \"bun x tsc-alias -w -p tsconfig.main.json\"" \ "bun x esbuild src/cli/api.ts --bundle --format=esm --platform=node --target=node20 --outfile=dist/cli/api.mjs --external:zod --external:commander --external:@trpc/server --watch" \ "bun x nodemon --watch dist/cli/index.js --watch dist/cli/server.js --delay 500ms --exec 'NODE_ENV=development node dist/cli/index.js server --host $(or $(BACKEND_HOST),localhost) --port $(or $(BACKEND_PORT),3000)'" \ @@ -173,7 +179,7 @@ endif start: node_modules/.installed build-main build-preload build-static ## Build and start Electron app - @NODE_ENV=development MUX_DISABLE_TELEMETRY=$(or $(MUX_DISABLE_TELEMETRY),1) bunx electron --remote-debugging-port=9222 . + @NODE_ENV=development MUX_DISABLE_TELEMETRY=$(if $(MUX_ENABLE_TELEMETRY_IN_DEV),,$(or $(MUX_DISABLE_TELEMETRY),1)) bunx electron --remote-debugging-port=9222 . ## Build targets (can run in parallel) build: node_modules/.installed src/version.ts build-renderer build-main build-preload build-icons build-static ## Build all targets diff --git a/src/browser/components/Settings/sections/ExperimentsSection.tsx b/src/browser/components/Settings/sections/ExperimentsSection.tsx index a4ecde4ef3..1285f30b47 100644 --- a/src/browser/components/Settings/sections/ExperimentsSection.tsx +++ b/src/browser/components/Settings/sections/ExperimentsSection.tsx @@ -1,5 +1,5 @@ -import React, { useCallback } from "react"; -import { useExperiment } from "@/browser/contexts/ExperimentsContext"; +import React, { useCallback, useMemo } from "react"; +import { useExperiment, useRemoteExperimentValue } from "@/browser/contexts/ExperimentsContext"; import { getExperimentList, EXPERIMENT_IDS, @@ -7,6 +7,7 @@ import { } from "@/common/constants/experiments"; import { Switch } from "@/browser/components/ui/switch"; import { useWorkspaceContext } from "@/browser/contexts/WorkspaceContext"; +import { useTelemetry } from "@/browser/hooks/useTelemetry"; interface ExperimentRowProps { experimentId: ExperimentId; @@ -17,14 +18,18 @@ interface ExperimentRowProps { function ExperimentRow(props: ExperimentRowProps) { const [enabled, setEnabled] = useExperiment(props.experimentId); - const { onToggle } = props; + const remote = useRemoteExperimentValue(props.experimentId); + const telemetry = useTelemetry(); + const { onToggle, experimentId } = props; const handleToggle = useCallback( (value: boolean) => { setEnabled(value); + // Track the override for analytics + telemetry.experimentOverridden(experimentId, remote?.value ?? null, value); onToggle?.(value); }, - [setEnabled, onToggle] + [setEnabled, telemetry, experimentId, remote?.value, onToggle] ); return ( @@ -43,9 +48,16 @@ function ExperimentRow(props: ExperimentRowProps) { } export function ExperimentsSection() { - const experiments = getExperimentList(); + const allExperiments = getExperimentList(); const { refreshWorkspaceMetadata } = useWorkspaceContext(); + // Only show user-overridable experiments (non-overridable ones are hidden since users can't change them) + const experiments = useMemo( + () => + allExperiments.filter((exp) => exp.showInSettings !== false && exp.userOverridable === true), + [allExperiments] + ); + // When post-compaction experiment is toggled, refresh metadata to fetch/clear bundled state const handlePostCompactionToggle = useCallback(() => { void refreshWorkspaceMetadata(); diff --git a/src/browser/contexts/ExperimentsContext.test.tsx b/src/browser/contexts/ExperimentsContext.test.tsx new file mode 100644 index 0000000000..4fd8fdb945 --- /dev/null +++ b/src/browser/contexts/ExperimentsContext.test.tsx @@ -0,0 +1,77 @@ +import { cleanup, render, waitFor } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; +import { GlobalWindow } from "happy-dom"; +import { ExperimentsProvider, useExperimentValue } from "./ExperimentsContext"; +import { EXPERIMENT_IDS } from "@/common/constants/experiments"; +import type { ExperimentValue } from "@/common/orpc/types"; +import type { APIClient } from "@/browser/contexts/API"; +import type { RecursivePartial } from "@/browser/testUtils"; + +let currentClientMock: RecursivePartial = {}; +void mock.module("@/browser/contexts/API", () => ({ + useAPI: () => ({ + api: currentClientMock as APIClient, + status: "connected" as const, + error: null, + }), + APIProvider: ({ children }: { children: React.ReactNode }) => children, +})); + +describe("ExperimentsProvider", () => { + beforeEach(() => { + globalThis.window = new GlobalWindow() as unknown as Window & typeof globalThis; + globalThis.document = globalThis.window.document; + globalThis.window.localStorage.clear(); + }); + + afterEach(() => { + cleanup(); + globalThis.window = undefined as unknown as Window & typeof globalThis; + globalThis.document = undefined as unknown as Document; + currentClientMock = {}; + }); + + test("polls getAll until remote variants are available", async () => { + let callCount = 0; + + const getAllMock = mock(() => { + callCount += 1; + + if (callCount === 1) { + return Promise.resolve({ + [EXPERIMENT_IDS.POST_COMPACTION_CONTEXT]: { value: null, source: "cache" }, + } satisfies Record); + } + + return Promise.resolve({ + [EXPERIMENT_IDS.POST_COMPACTION_CONTEXT]: { value: "test", source: "posthog" }, + } satisfies Record); + }); + + currentClientMock = { + experiments: { + getAll: getAllMock, + reload: mock(() => Promise.resolve()), + }, + }; + + function Observer() { + const enabled = useExperimentValue(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT); + return
{String(enabled)}
; + } + + const { getByTestId } = render( + + + + ); + + expect(getByTestId("enabled").textContent).toBe("false"); + + await waitFor(() => { + expect(getByTestId("enabled").textContent).toBe("true"); + }); + + expect(getAllMock.mock.calls.length).toBeGreaterThanOrEqual(2); + }); +}); diff --git a/src/browser/contexts/ExperimentsContext.tsx b/src/browser/contexts/ExperimentsContext.tsx index 0b2c4452b7..8a7ca6a34a 100644 --- a/src/browser/contexts/ExperimentsContext.tsx +++ b/src/browser/contexts/ExperimentsContext.tsx @@ -1,4 +1,12 @@ -import React, { createContext, useContext, useSyncExternalStore, useCallback } from "react"; +import React, { + createContext, + useContext, + useSyncExternalStore, + useCallback, + useEffect, + useRef, + useState, +} from "react"; import { type ExperimentId, EXPERIMENTS, @@ -6,6 +14,8 @@ import { getExperimentList, } from "@/common/constants/experiments"; import { getStorageChangeEvent } from "@/common/constants/events"; +import type { ExperimentValue } from "@/common/orpc/types"; +import { useAPI } from "@/browser/contexts/API"; /** * Subscribe to experiment changes for a specific experiment ID. @@ -28,21 +38,75 @@ function subscribeToExperiment(experimentId: ExperimentId, callback: () => void) } /** - * Get current experiment state from localStorage. + * Get explicit localStorage override for an experiment. + * Returns undefined if no value is set or parsing fails. */ -function getExperimentSnapshot(experimentId: ExperimentId): boolean { - const experiment = EXPERIMENTS[experimentId]; +function getExperimentOverrideSnapshot(experimentId: ExperimentId): boolean | undefined { const key = getExperimentKey(experimentId); try { const stored = window.localStorage.getItem(key); + // Check for literal "undefined" string defensively - this can occur if + // JSON.stringify(undefined) is accidentally stored (it returns "undefined") if (stored === null || stored === "undefined") { - return experiment.enabledByDefault; + return undefined; } - return JSON.parse(stored) as boolean; + + const parsed = JSON.parse(stored) as unknown; + return typeof parsed === "boolean" ? parsed : undefined; } catch { - return experiment.enabledByDefault; + return undefined; + } +} + +/** + * Get current experiment state from localStorage. + * Returns the stored value or the default if not set. + */ +function getExperimentSnapshot(experimentId: ExperimentId): boolean { + const experiment = EXPERIMENTS[experimentId]; + return getExperimentOverrideSnapshot(experimentId) ?? experiment.enabledByDefault; +} + +/** + * Check if user has explicitly set a local override for an experiment. + * Returns true if there's a value in localStorage (not using default). + */ +function hasLocalOverride(experimentId: ExperimentId): boolean { + return getExperimentOverrideSnapshot(experimentId) !== undefined; +} + +/** + * Convert PostHog experiment variant to boolean enabled state. + * For experiments with control/test variants, "test" means enabled. + */ +function getRemoteExperimentEnabled(value: string | boolean): boolean { + if (typeof value === "boolean") { + return value; } + return value === "test"; +} + +/** + * True when any remote experiment value is still pending a background PostHog refresh. + */ +function hasPendingRemoteExperimentValues( + remoteExperiments: Partial> +): boolean { + return Object.values(remoteExperiments).some( + (remote) => remote?.source === "cache" && remote.value === null + ); +} + +const REMOTE_EXPERIMENTS_POLL_INITIAL_DELAY_MS = 100; +const REMOTE_EXPERIMENTS_POLL_MAX_DELAY_MS = 5_000; +const REMOTE_EXPERIMENTS_POLL_MAX_ATTEMPTS = 8; + +function getRemoteExperimentsPollDelayMs(attempt: number): number { + return Math.min( + REMOTE_EXPERIMENTS_POLL_INITIAL_DELAY_MS * 2 ** attempt, + REMOTE_EXPERIMENTS_POLL_MAX_DELAY_MS + ); } /** @@ -70,6 +134,8 @@ function setExperimentState(experimentId: ExperimentId, enabled: boolean): void */ interface ExperimentsContextValue { setExperiment: (experimentId: ExperimentId, enabled: boolean) => void; + remoteExperiments: Partial> | null; + reloadRemoteExperiments: () => Promise; } const ExperimentsContext = createContext(null); @@ -83,8 +149,104 @@ export function ExperimentsProvider(props: { children: React.ReactNode }) { setExperimentState(experimentId, enabled); }, []); + const apiState = useAPI(); + const [remoteExperiments, setRemoteExperiments] = useState + > | null>(null); + + const loadRemoteExperiments = useCallback(async () => { + if (apiState.status !== "connected" || !apiState.api) { + setRemoteExperiments(null); + return; + } + + try { + const result = await apiState.api.experiments.getAll(); + setRemoteExperiments(result as Partial>); + } catch { + setRemoteExperiments(null); + } + }, [apiState.status, apiState.api]); + + const reloadRemoteExperiments = useCallback(async () => { + if (apiState.status !== "connected" || !apiState.api) { + setRemoteExperiments(null); + return; + } + + try { + await apiState.api.experiments.reload(); + } catch { + // Best effort + } + + await loadRemoteExperiments(); + }, [apiState.status, apiState.api, loadRemoteExperiments]); + + // On cold start, experiments.getAll can return { source: "cache", value: null } while + // ExperimentsService refreshes from PostHog in the background. Poll a few times so the + // renderer picks up remote variants without requiring a manual reload. + const remotePollTimeoutRef = useRef | null>(null); + const remotePollAttemptRef = useRef(0); + + const clearRemotePoll = useCallback(() => { + if (remotePollTimeoutRef.current === null) { + return; + } + + clearTimeout(remotePollTimeoutRef.current); + remotePollTimeoutRef.current = null; + }, []); + + useEffect(() => { + return () => { + clearRemotePoll(); + }; + }, [clearRemotePoll]); + + useEffect(() => { + if (apiState.status !== "connected" || !apiState.api) { + remotePollAttemptRef.current = 0; + clearRemotePoll(); + return; + } + + if (!remoteExperiments) { + remotePollAttemptRef.current = 0; + clearRemotePoll(); + return; + } + + if (!hasPendingRemoteExperimentValues(remoteExperiments)) { + remotePollAttemptRef.current = 0; + clearRemotePoll(); + return; + } + + if (remotePollTimeoutRef.current !== null) { + return; + } + + const attempt = remotePollAttemptRef.current; + if (attempt >= REMOTE_EXPERIMENTS_POLL_MAX_ATTEMPTS) { + return; + } + + const delayMs = getRemoteExperimentsPollDelayMs(attempt); + remotePollTimeoutRef.current = setTimeout(() => { + remotePollTimeoutRef.current = null; + remotePollAttemptRef.current += 1; + void loadRemoteExperiments(); + }, delayMs); + }, [apiState.status, apiState.api, remoteExperiments, clearRemotePoll, loadRemoteExperiments]); + useEffect(() => { + void loadRemoteExperiments(); + }, [loadRemoteExperiments]); + return ( - + {props.children} ); @@ -95,10 +257,16 @@ export function ExperimentsProvider(props: { children: React.ReactNode }) { * Uses useSyncExternalStore for efficient, selective re-renders. * Only re-renders when THIS specific experiment changes. * + * Resolution priority: + * - If userOverridable && user has explicitly set a local value → use local + * - If remote PostHog assignment exists → use remote + * - Otherwise → use local (which may be default) + * * @param experimentId - The experiment to subscribe to * @returns Whether the experiment is enabled */ export function useExperimentValue(experimentId: ExperimentId): boolean { + const experiment = EXPERIMENTS[experimentId]; const subscribe = useCallback( (callback: () => void) => subscribeToExperiment(experimentId, callback), [experimentId] @@ -106,6 +274,43 @@ export function useExperimentValue(experimentId: ExperimentId): boolean { const getSnapshot = useCallback(() => getExperimentSnapshot(experimentId), [experimentId]); + const localEnabled = useSyncExternalStore(subscribe, getSnapshot, getSnapshot); + + const context = useContext(ExperimentsContext); + const remote = context?.remoteExperiments?.[experimentId]; + + // User-overridable: local wins if explicitly set + if (experiment.userOverridable && hasLocalOverride(experimentId)) { + return localEnabled; + } + + // Remote assignment (if available and not disabled) + if (remote && remote.source !== "disabled" && remote.value !== null) { + return getRemoteExperimentEnabled(remote.value); + } + + // Fallback to local (which may be default) + return localEnabled; +} + +/** + * Hook to read only an explicit local override for an experiment. + * + * Returns `undefined` when the user has not explicitly set a value in localStorage. + * This is important for user-overridable experiments: the backend can then apply + * the PostHog assignment instead of treating the default value as a user choice. + */ +export function useExperimentOverrideValue(experimentId: ExperimentId): boolean | undefined { + const subscribe = useCallback( + (callback: () => void) => subscribeToExperiment(experimentId, callback), + [experimentId] + ); + + const getSnapshot = useCallback( + () => getExperimentOverrideSnapshot(experimentId), + [experimentId] + ); + return useSyncExternalStore(subscribe, getSnapshot, getSnapshot); } @@ -115,6 +320,11 @@ export function useExperimentValue(experimentId: ExperimentId): boolean { * * @returns Function to set experiment state */ + +export function useRemoteExperimentValue(experimentId: ExperimentId): ExperimentValue | null { + const context = useContext(ExperimentsContext); + return context?.remoteExperiments?.[experimentId] ?? null; +} export function useSetExperiment(): (experimentId: ExperimentId, enabled: boolean) => void { const context = useContext(ExperimentsContext); if (!context) { @@ -149,6 +359,8 @@ export function useExperiment(experimentId: ExperimentId): [boolean, (enabled: b */ export function useAllExperiments(): Record { const experiments = getExperimentList(); + const context = useContext(ExperimentsContext); + const remoteExperiments = context?.remoteExperiments; // Subscribe to all experiments const subscribe = useCallback( @@ -161,11 +373,29 @@ export function useAllExperiments(): Record { const getSnapshot = useCallback(() => { const result: Partial> = {}; + for (const exp of experiments) { - result[exp.id] = getExperimentSnapshot(exp.id); + const localValue = getExperimentSnapshot(exp.id); + const remote = remoteExperiments?.[exp.id]; + + // User-overridable: local wins if explicitly set + if (exp.userOverridable && hasLocalOverride(exp.id)) { + result[exp.id] = localValue; + continue; + } + + // Remote assignment (if available and not disabled) + if (remote && remote.source !== "disabled" && remote.value !== null) { + result[exp.id] = getRemoteExperimentEnabled(remote.value); + continue; + } + + // Fallback to local (which may be default) + result[exp.id] = localValue; } + return result as Record; - }, [experiments]); + }, [experiments, remoteExperiments]); return useSyncExternalStore(subscribe, getSnapshot, getSnapshot); } diff --git a/src/browser/hooks/useExperiments.test.ts b/src/browser/hooks/useExperiments.test.ts new file mode 100644 index 0000000000..74157fde66 --- /dev/null +++ b/src/browser/hooks/useExperiments.test.ts @@ -0,0 +1,53 @@ +/** + * Tests for isExperimentEnabled() + * + * Key invariant: + * - For user-overridable experiments, absence of a localStorage entry must be treated as + * "no explicit override" (undefined), so the backend can apply PostHog assignment. + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { GlobalWindow } from "happy-dom"; +import { EXPERIMENT_IDS, getExperimentKey } from "@/common/constants/experiments"; +import { isExperimentEnabled } from "./useExperiments"; + +describe("isExperimentEnabled", () => { + beforeEach(() => { + globalThis.window = new GlobalWindow() as unknown as Window & typeof globalThis; + globalThis.document = globalThis.window.document; + globalThis.window.localStorage.clear(); + }); + + afterEach(() => { + globalThis.window = undefined as unknown as Window & typeof globalThis; + globalThis.document = undefined as unknown as Document; + }); + + test("returns undefined when no local override exists for a user-overridable experiment", () => { + expect(isExperimentEnabled(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT)).toBeUndefined(); + }); + + test("returns boolean when local override exists", () => { + const key = getExperimentKey(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT); + + globalThis.window.localStorage.setItem(key, JSON.stringify(true)); + expect(isExperimentEnabled(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT)).toBe(true); + + globalThis.window.localStorage.setItem(key, JSON.stringify(false)); + expect(isExperimentEnabled(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT)).toBe(false); + }); + + test('treats literal "undefined" as no override', () => { + const key = getExperimentKey(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT); + + globalThis.window.localStorage.setItem(key, "undefined"); + expect(isExperimentEnabled(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT)).toBeUndefined(); + }); + + test("treats non-boolean stored value as no override", () => { + const key = getExperimentKey(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT); + + globalThis.window.localStorage.setItem(key, JSON.stringify("test")); + expect(isExperimentEnabled(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT)).toBeUndefined(); + }); +}); diff --git a/src/browser/hooks/useExperiments.ts b/src/browser/hooks/useExperiments.ts index cd1e334f56..702ed315b0 100644 --- a/src/browser/hooks/useExperiments.ts +++ b/src/browser/hooks/useExperiments.ts @@ -5,6 +5,7 @@ import { type ExperimentId, EXPERIMENTS, getExperimentKey } from "@/common/const export { useExperiment, useExperimentValue, + useExperimentOverrideValue, useSetExperiment, useAllExperiments, } from "@/browser/contexts/ExperimentsContext"; @@ -14,12 +15,28 @@ export { * Use when you need a one-time read (e.g., constructing send options at send time) * or outside of React components. * - * For reactive updates in React components, use useExperimentValue instead. + * For reactive updates in React components, use useExperimentValue (UI gating) or + * useExperimentOverrideValue (backend send options). + * + * IMPORTANT: For user-overridable experiments, returns `undefined` when no explicit + * localStorage override exists. This signals to the backend to use the PostHog + * assignment instead of treating the default value as a user choice. * * @param experimentId - The experiment to check - * @returns Whether the experiment is enabled + * @returns Whether the experiment is enabled, or undefined if backend should decide */ -export function isExperimentEnabled(experimentId: ExperimentId): boolean { +export function isExperimentEnabled(experimentId: ExperimentId): boolean | undefined { const experiment = EXPERIMENTS[experimentId]; - return readPersistedState(getExperimentKey(experimentId), experiment.enabledByDefault); + const key = getExperimentKey(experimentId); + + // For user-overridable experiments: only return a value if user explicitly set one. + // This allows the backend to use PostHog assignment when there's no override. + if (experiment.userOverridable) { + const stored = readPersistedState(key, undefined); + return typeof stored === "boolean" ? stored : undefined; + } + + // Non-overridable: always use default (these are local-only experiments) + const stored = readPersistedState(key, experiment.enabledByDefault); + return typeof stored === "boolean" ? stored : experiment.enabledByDefault; } diff --git a/src/browser/hooks/useSendMessageOptions.ts b/src/browser/hooks/useSendMessageOptions.ts index 9edae27e54..4cbb0f7bca 100644 --- a/src/browser/hooks/useSendMessageOptions.ts +++ b/src/browser/hooks/useSendMessageOptions.ts @@ -13,7 +13,7 @@ import { getSendOptionsFromStorage } from "@/browser/utils/messages/sendOptions" import { enforceThinkingPolicy } from "@/browser/utils/thinking/policy"; import { useProviderOptions } from "./useProviderOptions"; import type { GatewayState } from "./useGatewayModels"; -import { useExperimentValue } from "./useExperiments"; +import { useExperimentOverrideValue } from "./useExperiments"; import { EXPERIMENT_IDS } from "@/common/constants/experiments"; /** @@ -47,7 +47,7 @@ function constructSendMessageOptions( providerOptions: MuxProviderOptions, fallbackModel: string, gateway: GatewayState, - postCompactionContext: boolean + postCompactionContext: boolean | undefined ): SendMessageOptions { // Ensure model is always a valid string (defensive against corrupted localStorage) const rawModel = @@ -110,8 +110,9 @@ export function useSendMessageOptions(workspaceId: string): SendMessageOptionsWi // Subscribe to gateway state so we re-render when user toggles gateway const gateway = useGateway(); - // Subscribe to experiment state so toggles apply immediately - const postCompactionContext = useExperimentValue(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT); + // Subscribe to local override state so toggles apply immediately. + // If undefined, the backend will apply the PostHog assignment. + const postCompactionContext = useExperimentOverrideValue(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT); // Compute base model (canonical format) for UI components const rawModel = diff --git a/src/browser/hooks/useTelemetry.ts b/src/browser/hooks/useTelemetry.ts index 329e92e9fa..b6d270ba46 100644 --- a/src/browser/hooks/useTelemetry.ts +++ b/src/browser/hooks/useTelemetry.ts @@ -8,6 +8,7 @@ import { trackCommandUsed, trackVoiceTranscription, trackErrorOccurred, + trackExperimentOverridden, } from "@/common/telemetry"; import type { ErrorContext, @@ -35,6 +36,7 @@ import type { * telemetry.commandUsed(commandType); * telemetry.voiceTranscription(audioDurationSecs, success); * telemetry.errorOccurred(errorType, context); + * telemetry.experimentOverridden(experimentId, assignedVariant, userChoice); * ``` */ export function useTelemetry() { @@ -83,6 +85,13 @@ export function useTelemetry() { trackErrorOccurred(errorType, context); }, []); + const experimentOverridden = useCallback( + (experimentId: string, assignedVariant: string | boolean | null, userChoice: boolean) => { + trackExperimentOverridden(experimentId, assignedVariant, userChoice); + }, + [] + ); + return { workspaceSwitched, workspaceCreated, @@ -92,5 +101,6 @@ export function useTelemetry() { commandUsed, voiceTranscription, errorOccurred, + experimentOverridden, }; } diff --git a/src/cli/cli.test.ts b/src/cli/cli.test.ts index c825c5b927..fd4ed9740c 100644 --- a/src/cli/cli.test.ts +++ b/src/cli/cli.test.ts @@ -70,6 +70,7 @@ async function createTestServer(authToken?: string): Promise { serverService: services.serverService, mcpConfigService: services.mcpConfigService, mcpServerManager: services.mcpServerManager, + experimentsService: services.experimentsService, menuEventService: services.menuEventService, voiceService: services.voiceService, telemetryService: services.telemetryService, diff --git a/src/cli/server.test.ts b/src/cli/server.test.ts index 1883036ac9..8b4b0ba6d7 100644 --- a/src/cli/server.test.ts +++ b/src/cli/server.test.ts @@ -74,6 +74,7 @@ async function createTestServer(): Promise { mcpConfigService: services.mcpConfigService, mcpServerManager: services.mcpServerManager, menuEventService: services.menuEventService, + experimentsService: services.experimentsService, voiceService: services.voiceService, telemetryService: services.telemetryService, sessionUsageService: services.sessionUsageService, diff --git a/src/cli/server.ts b/src/cli/server.ts index 3040a37aba..c5f8acc03a 100644 --- a/src/cli/server.ts +++ b/src/cli/server.ts @@ -92,6 +92,7 @@ const mockWindow: BrowserWindow = { mcpServerManager: serviceContainer.mcpServerManager, voiceService: serviceContainer.voiceService, telemetryService: serviceContainer.telemetryService, + experimentsService: serviceContainer.experimentsService, sessionUsageService: serviceContainer.sessionUsageService, }; diff --git a/src/common/constants/experiments.ts b/src/common/constants/experiments.ts index f2f91367ee..72bad05fc2 100644 --- a/src/common/constants/experiments.ts +++ b/src/common/constants/experiments.ts @@ -17,6 +17,16 @@ export interface ExperimentDefinition { description: string; /** Default state - false means disabled by default */ enabledByDefault: boolean; + /** + * When true, user can override remote PostHog assignment via Settings toggle. + * When false (default), remote assignment is authoritative. + */ + userOverridable?: boolean; + /** + * When false, experiment is hidden from Settings → Experiments. + * Defaults to true. Use false for invisible A/B tests. + */ + showInSettings?: boolean; } /** @@ -29,6 +39,8 @@ export const EXPERIMENTS: Record = { name: "Post-Compaction Context", description: "Re-inject plan file and edited file diffs after compaction to preserve context", enabledByDefault: false, + userOverridable: true, // User can opt-out via Settings + showInSettings: true, }, }; diff --git a/src/common/orpc/schemas.ts b/src/common/orpc/schemas.ts index b344bab18d..efff170964 100644 --- a/src/common/orpc/schemas.ts +++ b/src/common/orpc/schemas.ts @@ -111,6 +111,8 @@ export { ProvidersConfigMapSchema, server, splashScreens, + experiments, + ExperimentValueSchema, telemetry, TelemetryEventSchema, terminal, diff --git a/src/common/orpc/schemas/api.ts b/src/common/orpc/schemas/api.ts index 0622f1f93a..3447c348e8 100644 --- a/src/common/orpc/schemas/api.ts +++ b/src/common/orpc/schemas/api.ts @@ -26,6 +26,22 @@ import { WorkspaceMCPOverridesSchema, } from "./mcp"; +// Experiments +export const ExperimentValueSchema = z.object({ + value: z.union([z.string(), z.boolean(), z.null()]), + source: z.enum(["posthog", "cache", "disabled"]), +}); + +export const experiments = { + getAll: { + input: z.void(), + output: z.record(z.string(), ExperimentValueSchema), + }, + reload: { + input: z.void(), + output: z.void(), + }, +}; // Re-export telemetry schemas export { telemetry, TelemetryEventSchema } from "./telemetry"; diff --git a/src/common/orpc/schemas/telemetry.ts b/src/common/orpc/schemas/telemetry.ts index 2b46196d2c..9997f6496f 100644 --- a/src/common/orpc/schemas/telemetry.ts +++ b/src/common/orpc/schemas/telemetry.ts @@ -97,6 +97,12 @@ const ErrorOccurredPropertiesSchema = z.object({ context: ErrorContextSchema, }); +const ExperimentOverriddenPropertiesSchema = z.object({ + experimentId: z.string(), + assignedVariant: z.union([z.string(), z.boolean(), z.null()]), + userChoice: z.boolean(), +}); + // Union of all telemetry events export const TelemetryEventSchema = z.discriminatedUnion("event", [ z.object({ @@ -135,6 +141,10 @@ export const TelemetryEventSchema = z.discriminatedUnion("event", [ event: z.literal("error_occurred"), properties: ErrorOccurredPropertiesSchema, }), + z.object({ + event: z.literal("experiment_overridden"), + properties: ExperimentOverriddenPropertiesSchema, + }), ]); // API schemas - only track endpoint, enabled state controlled by env var diff --git a/src/common/orpc/types.ts b/src/common/orpc/types.ts index 51a7759141..5201757219 100644 --- a/src/common/orpc/types.ts +++ b/src/common/orpc/types.ts @@ -34,6 +34,9 @@ export type FrontendWorkspaceMetadataSchemaType = z.infer< typeof schemas.FrontendWorkspaceMetadataSchema >; +// Experiment types (single source of truth - derived from schemas) +export type ExperimentValue = z.infer; + // Type guards for common chat message variants export function isCaughtUpMessage(msg: WorkspaceChatMessage): msg is CaughtUpMessage { return (msg as { type?: string }).type === "caught-up"; diff --git a/src/common/telemetry/index.ts b/src/common/telemetry/index.ts index 5eb740abf2..4cdfa3a5c2 100644 --- a/src/common/telemetry/index.ts +++ b/src/common/telemetry/index.ts @@ -24,6 +24,7 @@ export { trackCommandUsed, trackVoiceTranscription, trackErrorOccurred, + trackExperimentOverridden, } from "./tracking"; // Utility for converting RuntimeConfig to telemetry-safe runtime type diff --git a/src/common/telemetry/payload.ts b/src/common/telemetry/payload.ts index a266983c5e..1dfea2103f 100644 --- a/src/common/telemetry/payload.ts +++ b/src/common/telemetry/payload.ts @@ -188,6 +188,19 @@ export interface ErrorOccurredPayload { context: ErrorContext; } +/** + * Experiment override event - tracks when users manually toggle experiments + * This helps measure opt-out rates and understand user preferences + */ +export interface ExperimentOverriddenPayload { + /** Experiment identifier (e.g., 'post-compaction-context') */ + experimentId: string; + /** The variant PostHog assigned (null if not remote-controlled) */ + assignedVariant: string | boolean | null; + /** What the user chose (true = enabled, false = disabled) */ + userChoice: boolean; +} + /** * Union type of all telemetry event payloads * Frontend sends these; backend adds BaseTelemetryProperties before forwarding to PostHog @@ -201,4 +214,5 @@ export type TelemetryEventPayload = | { event: "provider_configured"; properties: ProviderConfiguredPayload } | { event: "command_used"; properties: CommandUsedPayload } | { event: "voice_transcription"; properties: VoiceTranscriptionPayload } - | { event: "error_occurred"; properties: ErrorOccurredPayload }; + | { event: "error_occurred"; properties: ErrorOccurredPayload } + | { event: "experiment_overridden"; properties: ExperimentOverriddenPayload }; diff --git a/src/common/telemetry/tracking.ts b/src/common/telemetry/tracking.ts index c033b804e6..1de807c3f6 100644 --- a/src/common/telemetry/tracking.ts +++ b/src/common/telemetry/tracking.ts @@ -162,3 +162,20 @@ export function trackErrorOccurred( properties: { errorType, context }, }); } + +/** + * Track experiment override - when a user manually toggles an experiment + * @param experimentId - The experiment identifier + * @param assignedVariant - What PostHog assigned (null if not remote-controlled) + * @param userChoice - What the user chose (true = enabled, false = disabled) + */ +export function trackExperimentOverridden( + experimentId: string, + assignedVariant: string | boolean | null, + userChoice: boolean +): void { + trackEvent({ + event: "experiment_overridden", + properties: { experimentId, assignedVariant, userChoice }, + }); +} diff --git a/src/desktop/main.ts b/src/desktop/main.ts index f4873fee4a..34505e0d0e 100644 --- a/src/desktop/main.ts +++ b/src/desktop/main.ts @@ -338,6 +338,7 @@ async function loadServices(): Promise { menuEventService: services.menuEventService, voiceService: services.voiceService, telemetryService: services.telemetryService, + experimentsService: services.experimentsService, sessionUsageService: services.sessionUsageService, }; diff --git a/src/node/orpc/context.ts b/src/node/orpc/context.ts index b61efd9034..79e38731f7 100644 --- a/src/node/orpc/context.ts +++ b/src/node/orpc/context.ts @@ -13,6 +13,7 @@ import type { ServerService } from "@/node/services/serverService"; import type { MenuEventService } from "@/node/services/menuEventService"; import type { VoiceService } from "@/node/services/voiceService"; import type { MCPConfigService } from "@/node/services/mcpConfigService"; +import type { ExperimentsService } from "@/node/services/experimentsService"; import type { MCPServerManager } from "@/node/services/mcpServerManager"; import type { TelemetryService } from "@/node/services/telemetryService"; import type { SessionUsageService } from "@/node/services/sessionUsageService"; @@ -34,6 +35,7 @@ export interface ORPCContext { mcpConfigService: MCPConfigService; mcpServerManager: MCPServerManager; telemetryService: TelemetryService; + experimentsService: ExperimentsService; sessionUsageService: SessionUsageService; headers?: IncomingHttpHeaders; } diff --git a/src/node/orpc/router.ts b/src/node/orpc/router.ts index aa1141483d..60082c3673 100644 --- a/src/node/orpc/router.ts +++ b/src/node/orpc/router.ts @@ -977,6 +977,20 @@ export const router = (authToken?: string) => { return context.voiceService.transcribe(input.audioBase64); }), }, + experiments: { + getAll: t + .input(schemas.experiments.getAll.input) + .output(schemas.experiments.getAll.output) + .handler(({ context }) => { + return context.experimentsService.getAll(); + }), + reload: t + .input(schemas.experiments.reload.input) + .output(schemas.experiments.reload.output) + .handler(async ({ context }) => { + await context.experimentsService.refreshAll(); + }), + }, debug: { triggerStreamError: t .input(schemas.debug.triggerStreamError.input) diff --git a/src/node/services/backgroundProcessManager.test.ts b/src/node/services/backgroundProcessManager.test.ts index 1e8609f89a..c7164abdc9 100644 --- a/src/node/services/backgroundProcessManager.test.ts +++ b/src/node/services/backgroundProcessManager.test.ts @@ -397,33 +397,33 @@ describe("BackgroundProcessManager", () => { describe("getOutput", () => { it("should return stdout from a running process", async () => { - // Spawn a process that writes output over time - // Use longer sleep and explicit flush to ensure output is written to file + // Spawn a process that writes output in two phases. + // Use a file-gated barrier rather than timing sleeps to avoid CI flakiness. + const triggerFile = path.join(bgOutputDir, `trigger-${Date.now()}`); + const result = await manager.spawn( runtime, testWorkspaceId, - "echo 'line 1'; sleep 0.3; echo 'line 2'", + `echo 'line 1'; while [ ! -f ${triggerFile} ]; do sleep 0.05; done; echo 'line 2'`, { cwd: process.cwd(), displayName: "test" } ); expect(result.success).toBe(true); if (!result.success) return; - // Wait for first line to be written and flushed (increased for CI reliability) - await new Promise((resolve) => setTimeout(resolve, 200)); - - // Get output - should have at least the first line - const output1 = await manager.getOutput(result.processId); + // Get output - wait up to 1s for the first line + const output1 = await manager.getOutput(result.processId, undefined, undefined, 1); expect(output1.success).toBe(true); if (!output1.success) return; expect(output1.output).toContain("line 1"); + expect(output1.output).not.toContain("line 2"); - // Wait for second line (sleep 0.3s + buffer for CI) - await new Promise((resolve) => setTimeout(resolve, 500)); + // Unblock the process so it can emit the second line + await fs.writeFile(triggerFile, "go", "utf-8"); - // Get output again - should have incremental output (line 2) - const output2 = await manager.getOutput(result.processId); + // Get output again - wait up to 1s for incremental output (line 2) + const output2 = await manager.getOutput(result.processId, undefined, undefined, 1); expect(output2.success).toBe(true); if (!output2.success) return; diff --git a/src/node/services/backgroundProcessManager.ts b/src/node/services/backgroundProcessManager.ts index 6904d75de5..8616e71a13 100644 --- a/src/node/services/backgroundProcessManager.ts +++ b/src/node/services/backgroundProcessManager.ts @@ -548,21 +548,41 @@ export class BackgroundProcessManager extends EventEmitter 0 - : completeLines.length > 0 || incompleteLine.length > 0; + : completeLines.length > 0; // Return immediately if: // 1. We have meaningful output (after filtering if filter_exclude is set) - // 2. Process is no longer running (exited/killed/failed) - flush buffer - // 3. Timeout elapsed - // 4. Abort signal received (user sent a new message) - if (hasMeaningfulOutput || currentStatus !== "running") { + // 2. Timeout elapsed + // 3. Abort signal received (user sent a new message) + if (hasMeaningfulOutput) { + break; + } + + // If the process is no longer running (exited/killed/failed), do one last read + // to avoid dropping output that arrives between our readOutput() call and + // the status refresh. + if (currentStatus !== "running") { + while (true) { + const finalRead = await proc.handle.readOutput(proc.outputBytesRead); + if (finalRead.content.length === 0) { + break; + } + + // Defensive: avoid infinite loops if a handle returns inconsistent offsets. + if (finalRead.newOffset <= proc.outputBytesRead) { + break; + } + + accumulatedRaw += finalRead.content; + proc.outputBytesRead = finalRead.newOffset; + } + break; } diff --git a/src/node/services/experimentsService.test.ts b/src/node/services/experimentsService.test.ts new file mode 100644 index 0000000000..3e205236b9 --- /dev/null +++ b/src/node/services/experimentsService.test.ts @@ -0,0 +1,130 @@ +import { describe, expect, test, beforeEach, afterEach, mock } from "bun:test"; +import { ExperimentsService } from "./experimentsService"; +import { EXPERIMENT_IDS } from "@/common/constants/experiments"; +import type { TelemetryService } from "./telemetryService"; +import type { PostHog } from "posthog-node"; +import * as fs from "fs/promises"; +import * as os from "os"; +import * as path from "path"; + +describe("ExperimentsService", () => { + let tempDir: string; + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "mux-experiments-test-")); + }); + + afterEach(async () => { + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + test("loads cached experiment values from disk and exposes them", async () => { + const cacheFilePath = path.join(tempDir, "feature_flags.json"); + await fs.writeFile( + cacheFilePath, + JSON.stringify( + { + version: 1, + experiments: { + [EXPERIMENT_IDS.POST_COMPACTION_CONTEXT]: { + value: "test", + fetchedAtMs: Date.now(), + }, + }, + }, + null, + 2 + ), + "utf-8" + ); + + const setFeatureFlagVariant = mock(() => undefined); + const fakePostHog = { + getFeatureFlag: mock(() => Promise.resolve("test")), + } as unknown as PostHog; + + const telemetryService = { + getPostHogClient: mock(() => fakePostHog), + getDistinctId: mock(() => "distinct-id"), + setFeatureFlagVariant, + } as unknown as TelemetryService; + + const service = new ExperimentsService({ + telemetryService, + muxHome: tempDir, + cacheTtlMs: 60 * 60 * 1000, + }); + + await service.initialize(); + + const values = service.getAll(); + expect(values[EXPERIMENT_IDS.POST_COMPACTION_CONTEXT]).toEqual({ + value: "test", + source: "cache", + }); + + expect(setFeatureFlagVariant).toHaveBeenCalledWith( + EXPERIMENT_IDS.POST_COMPACTION_CONTEXT, + "test" + ); + }); + + test("refreshExperiment updates cache and writes it to disk", async () => { + const setFeatureFlagVariant = mock(() => undefined); + const fakePostHog = { + getFeatureFlag: mock(() => Promise.resolve("test")), + } as unknown as PostHog; + + const telemetryService = { + getPostHogClient: mock(() => fakePostHog), + getDistinctId: mock(() => "distinct-id"), + setFeatureFlagVariant, + } as unknown as TelemetryService; + + const service = new ExperimentsService({ + telemetryService, + muxHome: tempDir, + cacheTtlMs: 0, + }); + + await service.initialize(); + await service.refreshExperiment(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT); + + const value = service.getExperimentValue(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT); + expect(value.value).toBe("test"); + expect(value.source).toBe("posthog"); + + const cacheFilePath = path.join(tempDir, "feature_flags.json"); + const disk = JSON.parse(await fs.readFile(cacheFilePath, "utf-8")) as unknown; + expect(typeof disk).toBe("object"); + + expect((disk as { version: unknown }).version).toBe(1); + expect((disk as { experiments: Record }).experiments).toHaveProperty( + EXPERIMENT_IDS.POST_COMPACTION_CONTEXT + ); + + expect(setFeatureFlagVariant).toHaveBeenCalledWith( + EXPERIMENT_IDS.POST_COMPACTION_CONTEXT, + "test" + ); + }); + + test("returns disabled when telemetry is disabled", async () => { + const telemetryService = { + getPostHogClient: mock(() => null), + getDistinctId: mock(() => null), + setFeatureFlagVariant: mock(() => undefined), + } as unknown as TelemetryService; + + const service = new ExperimentsService({ telemetryService, muxHome: tempDir }); + await service.initialize(); + + const values = service.getAll(); + expect(values[EXPERIMENT_IDS.POST_COMPACTION_CONTEXT]).toEqual({ + value: null, + source: "disabled", + }); + + expect(service.isExperimentEnabled(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT)).toBe(false); + }); +}); diff --git a/src/node/services/experimentsService.ts b/src/node/services/experimentsService.ts new file mode 100644 index 0000000000..507661e0c2 --- /dev/null +++ b/src/node/services/experimentsService.ts @@ -0,0 +1,306 @@ +import assert from "@/common/utils/assert"; +import { EXPERIMENTS, type ExperimentId } from "@/common/constants/experiments"; +import { getMuxHome } from "@/common/constants/paths"; +import type { ExperimentValue } from "@/common/orpc/types"; +import { log } from "@/node/services/log"; +import type { TelemetryService } from "@/node/services/telemetryService"; +import * as fs from "fs/promises"; +import * as path from "path"; + +export type { ExperimentValue }; + +interface CachedVariant { + value: string | boolean; + fetchedAtMs: number; + source: "posthog" | "cache"; +} + +interface ExperimentsCacheFile { + version: 1; + experiments: Record; +} + +const CACHE_FILE_NAME = "feature_flags.json"; +const CACHE_FILE_VERSION = 1; +const DEFAULT_CACHE_TTL_MS = 10 * 60 * 1000; + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null; +} + +/** + * Backend experiments service. + * + * Evaluates PostHog feature flags in the main process (via posthog-node) and exposes + * the current assignments to the renderer via oRPC. + * + * Design goals: + * - Never block user flows on network calls (use cached values and refresh in background) + * - Fail closed (unknown = control/disabled) + * - Avoid calling PostHog when telemetry is disabled + */ +export class ExperimentsService { + private readonly telemetryService: TelemetryService; + private readonly muxHome: string; + private readonly cacheFilePath: string; + private readonly cacheTtlMs: number; + + private readonly cachedVariants = new Map(); + private readonly refreshInFlight = new Map>(); + + private cacheLoaded = false; + + constructor(options: { + telemetryService: TelemetryService; + muxHome?: string; + cacheTtlMs?: number; + }) { + this.telemetryService = options.telemetryService; + this.muxHome = options.muxHome ?? getMuxHome(); + this.cacheFilePath = path.join(this.muxHome, CACHE_FILE_NAME); + this.cacheTtlMs = options.cacheTtlMs ?? DEFAULT_CACHE_TTL_MS; + } + + async initialize(): Promise { + if (this.cacheLoaded) { + return; + } + + await this.loadCacheFromDisk(); + this.cacheLoaded = true; + + // Populate telemetry properties from cache immediately so variant breakdowns + // are present even before a background refresh completes. + for (const [experimentId, cached] of this.cachedVariants) { + this.telemetryService.setFeatureFlagVariant(this.getFlagKey(experimentId), cached.value); + } + + // Refresh in background (best effort). We only refresh values that are stale or missing + // to avoid unnecessary network calls during startup. + if (this.isRemoteEvaluationEnabled()) { + for (const experimentId of Object.keys(EXPERIMENTS) as ExperimentId[]) { + this.maybeRefreshInBackground(experimentId); + } + } + } + + isRemoteEvaluationEnabled(): boolean { + return ( + this.telemetryService.getPostHogClient() !== null && + this.telemetryService.getDistinctId() !== null + ); + } + + /** + * Return current values for all known experiments. + * This is used to render Settings → Experiments. + */ + getAll(): Record { + const result: Partial> = {}; + + for (const experimentId of Object.keys(EXPERIMENTS) as ExperimentId[]) { + result[experimentId] = this.getExperimentValue(experimentId); + } + + return result as Record; + } + + getExperimentValue(experimentId: ExperimentId): ExperimentValue { + assert(experimentId in EXPERIMENTS, `Unknown experimentId: ${experimentId}`); + + if (!this.isRemoteEvaluationEnabled()) { + return { value: null, source: "disabled" }; + } + + const cached = this.cachedVariants.get(experimentId); + if (!cached) { + // No cached value yet. Fail closed, but kick off a background refresh. + this.maybeRefreshInBackground(experimentId); + return { value: null, source: "cache" }; + } + + this.maybeRefreshInBackground(experimentId); + return { value: cached.value, source: cached.source }; + } + + /** + * Convert an experiment assignment to a boolean gate. + * + * NOTE: This intentionally does not block on network calls. + */ + isExperimentEnabled(experimentId: ExperimentId): boolean { + const value = this.getExperimentValue(experimentId).value; + + // PostHog can return either boolean flags or string variants. + if (typeof value === "boolean") { + return value; + } + + if (typeof value === "string") { + // For now, treat variant "test" as enabled for experiments with control/test variants. + // If we add experiments with different variant semantics, add a mapping per experiment. + return value === "test"; + } + + return false; + } + + async refreshAll(): Promise { + await this.ensureInitialized(); + + if (!this.isRemoteEvaluationEnabled()) { + return; + } + + await Promise.all( + (Object.keys(EXPERIMENTS) as ExperimentId[]).map(async (experimentId) => { + await this.refreshExperiment(experimentId); + }) + ); + } + + async refreshExperiment(experimentId: ExperimentId): Promise { + await this.ensureInitialized(); + assert(experimentId in EXPERIMENTS, `Unknown experimentId: ${experimentId}`); + + if (!this.isRemoteEvaluationEnabled()) { + return; + } + + const existing = this.refreshInFlight.get(experimentId); + if (existing) { + return existing; + } + + const promise = this.refreshExperimentImpl(experimentId).finally(() => { + this.refreshInFlight.delete(experimentId); + }); + + this.refreshInFlight.set(experimentId, promise); + return promise; + } + + private async refreshExperimentImpl(experimentId: ExperimentId): Promise { + const client = this.telemetryService.getPostHogClient(); + const distinctId = this.telemetryService.getDistinctId(); + assert(client, "PostHog client must exist when remote evaluation is enabled"); + assert(distinctId, "distinctId must exist when remote evaluation is enabled"); + + const flagKey = this.getFlagKey(experimentId); + + try { + const value = await client.getFeatureFlag(flagKey, distinctId); + if (typeof value !== "string" && typeof value !== "boolean") { + return; + } + + const cached: CachedVariant = { + value, + fetchedAtMs: Date.now(), + source: "posthog", + }; + + this.cachedVariants.set(experimentId, cached); + this.telemetryService.setFeatureFlagVariant(flagKey, value); + + await this.writeCacheToDisk(); + } catch (error) { + log.debug("Failed to refresh experiment from PostHog", { + experimentId, + error: error instanceof Error ? error.message : String(error), + }); + } + } + + private maybeRefreshInBackground(experimentId: ExperimentId): void { + const cached = this.cachedVariants.get(experimentId); + if (!cached) { + void this.refreshExperiment(experimentId); + return; + } + + if (Date.now() - cached.fetchedAtMs > this.cacheTtlMs) { + void this.refreshExperiment(experimentId); + } + } + + private getFlagKey(experimentId: ExperimentId): string { + // Today, our experiment IDs are already PostHog flag keys. + // If that ever changes, this is the single mapping point. + return experimentId; + } + + private async ensureInitialized(): Promise { + if (this.cacheLoaded) { + return; + } + + await this.initialize(); + assert(this.cacheLoaded, "ExperimentsService failed to initialize"); + } + + private async loadCacheFromDisk(): Promise { + try { + const raw = await fs.readFile(this.cacheFilePath, "utf-8"); + const parsed = JSON.parse(raw) as unknown; + + if (!isRecord(parsed)) { + return; + } + + const version = parsed.version; + const experiments = parsed.experiments; + + if (version !== CACHE_FILE_VERSION || !isRecord(experiments)) { + return; + } + + for (const [key, value] of Object.entries(experiments)) { + if (!(key in EXPERIMENTS) || !isRecord(value)) { + continue; + } + + const fetchedAtMs = value.fetchedAtMs; + const variant = value.value; + + if (typeof fetchedAtMs !== "number" || !Number.isFinite(fetchedAtMs)) { + continue; + } + + if (typeof variant !== "string" && typeof variant !== "boolean") { + continue; + } + + this.cachedVariants.set(key as ExperimentId, { + value: variant, + fetchedAtMs, + source: "cache", + }); + } + } catch { + // Ignore missing/corrupt cache + } + } + + private async writeCacheToDisk(): Promise { + try { + const experiments: ExperimentsCacheFile["experiments"] = {}; + for (const [experimentId, cached] of this.cachedVariants) { + experiments[experimentId] = { + value: cached.value, + fetchedAtMs: cached.fetchedAtMs, + }; + } + + const payload: ExperimentsCacheFile = { + version: CACHE_FILE_VERSION, + experiments, + }; + + await fs.mkdir(this.muxHome, { recursive: true }); + await fs.writeFile(this.cacheFilePath, JSON.stringify(payload, null, 2), "utf-8"); + } catch { + // Ignore cache persistence failures + } + } +} diff --git a/src/node/services/serviceContainer.ts b/src/node/services/serviceContainer.ts index 63b63447df..8701e8007e 100644 --- a/src/node/services/serviceContainer.ts +++ b/src/node/services/serviceContainer.ts @@ -20,6 +20,7 @@ import { ServerService } from "@/node/services/serverService"; import { MenuEventService } from "@/node/services/menuEventService"; import { VoiceService } from "@/node/services/voiceService"; import { TelemetryService } from "@/node/services/telemetryService"; +import { ExperimentsService } from "@/node/services/experimentsService"; import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; import { MCPConfigService } from "@/node/services/mcpConfigService"; import { MCPServerManager } from "@/node/services/mcpServerManager"; @@ -51,6 +52,7 @@ export class ServiceContainer { public readonly mcpConfigService: MCPConfigService; public readonly mcpServerManager: MCPServerManager; public readonly telemetryService: TelemetryService; + public readonly experimentsService: ExperimentsService; public readonly sessionUsageService: SessionUsageService; private readonly initStateManager: InitStateManager; private readonly extensionMetadata: ExtensionMetadataService; @@ -114,12 +116,18 @@ export class ServiceContainer { this.menuEventService = new MenuEventService(); this.voiceService = new VoiceService(config); this.telemetryService = new TelemetryService(config.rootDir); + this.experimentsService = new ExperimentsService({ + telemetryService: this.telemetryService, + muxHome: config.rootDir, + }); + this.workspaceService.setExperimentsService(this.experimentsService); } async initialize(): Promise { await this.extensionMetadata.initialize(); // Initialize telemetry service await this.telemetryService.initialize(); + await this.experimentsService.initialize(); // Start idle compaction checker this.idleCompactionService.start(); } diff --git a/src/node/services/telemetryService.featureFlags.test.ts b/src/node/services/telemetryService.featureFlags.test.ts new file mode 100644 index 0000000000..feecee4ce3 --- /dev/null +++ b/src/node/services/telemetryService.featureFlags.test.ts @@ -0,0 +1,93 @@ +import { describe, expect, test, beforeEach, afterEach, mock } from "bun:test"; +import { TelemetryService } from "./telemetryService"; +import type { TelemetryEventPayload } from "@/common/telemetry/payload"; +import * as fs from "fs/promises"; +import * as os from "os"; +import * as path from "path"; + +describe("TelemetryService feature flag properties", () => { + let tempDir: string; + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "mux-telemetry-test-")); + }); + + afterEach(async () => { + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + test("capture includes $feature/ properties when set", () => { + // The capture method checks isTelemetryDisabledByEnv which checks NODE_ENV=test, + // JEST_WORKER_ID, VITEST, CI, etc. We need to temporarily clear these for the test. + const savedEnv = { + NODE_ENV: process.env.NODE_ENV, + JEST_WORKER_ID: process.env.JEST_WORKER_ID, + VITEST: process.env.VITEST, + CI: process.env.CI, + GITHUB_ACTIONS: process.env.GITHUB_ACTIONS, + MUX_DISABLE_TELEMETRY: process.env.MUX_DISABLE_TELEMETRY, + MUX_E2E: process.env.MUX_E2E, + TEST_INTEGRATION: process.env.TEST_INTEGRATION, + }; + + // Clear all telemetry-disabling env vars + process.env.NODE_ENV = "production"; + delete process.env.JEST_WORKER_ID; + delete process.env.VITEST; + delete process.env.CI; + delete process.env.GITHUB_ACTIONS; + delete process.env.MUX_DISABLE_TELEMETRY; + delete process.env.MUX_E2E; + delete process.env.TEST_INTEGRATION; + + try { + const telemetry = new TelemetryService(tempDir); + + const capture = mock((_args: unknown) => undefined); + + // NOTE: TelemetryService only checks that client + distinctId are set. + // We set them directly to avoid any real network calls. + // @ts-expect-error - Accessing private property for test + telemetry.client = { capture }; + // @ts-expect-error - Accessing private property for test + telemetry.distinctId = "distinct-id"; + + telemetry.setFeatureFlagVariant("post-compaction-context", "test"); + + const payload: TelemetryEventPayload = { + event: "message_sent", + properties: { + workspaceId: "workspace-id", + model: "test-model", + mode: "exec", + message_length_b2: 128, + runtimeType: "local", + frontendPlatform: { + userAgent: "ua", + platform: "platform", + }, + thinkingLevel: "off", + }, + }; + + telemetry.capture(payload); + + expect(capture).toHaveBeenCalled(); + + const call = capture.mock.calls[0]?.[0] as + | { properties?: Record } + | undefined; + expect(call?.properties).toBeDefined(); + expect(call?.properties?.["$feature/post-compaction-context"]).toBe("test"); + } finally { + // Restore all env vars + for (const [key, value] of Object.entries(savedEnv)) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + } + }); +}); diff --git a/src/node/services/telemetryService.ts b/src/node/services/telemetryService.ts index 39c7225d7f..87248fef4e 100644 --- a/src/node/services/telemetryService.ts +++ b/src/node/services/telemetryService.ts @@ -12,6 +12,7 @@ * Uses posthog-node which batches events and flushes asynchronously. */ +import assert from "@/common/utils/assert"; import { PostHog } from "posthog-node"; import { randomUUID } from "crypto"; import * as fs from "fs/promises"; @@ -144,8 +145,44 @@ function getVersionString(): string { export class TelemetryService { private client: PostHog | null = null; private distinctId: string | null = null; + private featureFlagVariants: Record = {}; private readonly muxHome: string; + getPostHogClient(): PostHog | null { + return this.client; + } + + getDistinctId(): string | null { + return this.distinctId; + } + + /** + * Set the current PostHog feature flag/experiment assignment. + * + * This is used to attach `$feature/` properties to all telemetry events so + * PostHog can break down metrics by experiment variant (required for server-side capture). + */ + setFeatureFlagVariant(flagKey: string, variant: string | boolean | null): void { + assert(typeof flagKey === "string", "flagKey must be a string"); + const trimmed = flagKey.trim(); + assert(trimmed.length > 0, "flagKey must not be empty"); + + const key = `$feature/${trimmed}`; + + if (variant === null) { + // Removing the property avoids emitting null values which can pollute breakdowns. + // Note: This is safe even if telemetry is disabled. + delete this.featureFlagVariants[key]; + return; + } + + assert( + typeof variant === "string" || typeof variant === "boolean", + "variant must be a string | boolean | null" + ); + + this.featureFlagVariants[key] = variant; + } constructor(muxHome?: string) { this.muxHome = muxHome ?? getMuxHome(); } @@ -178,7 +215,7 @@ export class TelemetryService { this.client = new PostHog(DEFAULT_POSTHOG_KEY, { host: DEFAULT_POSTHOG_HOST, - // Disable feature flags since we don't use them + // Avoid geo-IP enrichment (we don't need coarse location for mux telemetry) disableGeoip: true, }); @@ -219,13 +256,14 @@ export class TelemetryService { /** * Get base properties included with all events */ - private getBaseProperties(): BaseTelemetryProperties { + private getBaseProperties(): BaseTelemetryProperties & Record { return { version: getVersionString(), backend_platform: process.platform, electronVersion: process.versions.electron ?? "unknown", nodeVersion: process.versions.node ?? "unknown", bunVersion: process.versions.bun ?? "unknown", + ...this.featureFlagVariants, }; } diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index 4cd8f10c53..c5a34892cd 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -13,6 +13,8 @@ import type { PartialService } from "@/node/services/partialService"; import type { AIService } from "@/node/services/aiService"; import type { InitStateManager } from "@/node/services/initStateManager"; import type { ExtensionMetadataService } from "@/node/services/ExtensionMetadataService"; +import type { ExperimentsService } from "@/node/services/experimentsService"; +import { EXPERIMENT_IDS, EXPERIMENTS } from "@/common/constants/experiments"; import type { MCPServerManager } from "@/node/services/mcpServerManager"; import { createRuntime, IncompatibleRuntimeError } from "@/node/runtime/runtimeFactory"; import { validateWorkspaceName } from "@/common/utils/validation/workspaceValidation"; @@ -118,6 +120,7 @@ export class WorkspaceService extends EventEmitter { this.setupMetadataListeners(); } + private experimentsService?: ExperimentsService; private mcpServerManager?: MCPServerManager; // Optional terminal service for cleanup on workspace removal private terminalService?: TerminalService; @@ -130,6 +133,10 @@ export class WorkspaceService extends EventEmitter { this.mcpServerManager = manager; } + setExperimentsService(experimentsService: ExperimentsService): void { + this.experimentsService = experimentsService; + } + /** * Set the terminal service for cleanup on workspace removal. */ @@ -680,7 +687,27 @@ export class WorkspaceService extends EventEmitter { try { const metadata = await this.config.getAllWorkspaceMetadata(); - if (!options?.includePostCompaction) { + // For list(), treat includePostCompaction as an explicit frontend override when provided. + // If it's undefined (e.g., user hasn't overridden), fall back to PostHog assignment. + const postCompactionExperiment = EXPERIMENTS[EXPERIMENT_IDS.POST_COMPACTION_CONTEXT]; + let includePostCompaction: boolean; + if ( + postCompactionExperiment.userOverridable && + options?.includePostCompaction !== undefined + ) { + // User-overridable: trust frontend value + includePostCompaction = options.includePostCompaction; + } else if (this.experimentsService?.isRemoteEvaluationEnabled() === true) { + // Remote evaluation: use PostHog assignment + includePostCompaction = this.experimentsService.isExperimentEnabled( + EXPERIMENT_IDS.POST_COMPACTION_CONTEXT + ); + } else { + // Fallback to frontend value or false + includePostCompaction = options?.includePostCompaction === true; + } + + if (!includePostCompaction) { return metadata; } @@ -995,7 +1022,39 @@ export class WorkspaceService extends EventEmitter { void this.updateRecencyTimestamp(workspaceId, messageTimestamp); } - if (this.aiService.isStreaming(workspaceId) && !options?.editMessageId) { + // Experiments: resolve flags respecting userOverridable setting. + // - If userOverridable && frontend provides a value (explicit override) → use frontend value + // - Else if remote evaluation enabled → use PostHog assignment + // - Else → use frontend value (dev fallback) or default + const postCompactionExperiment = EXPERIMENTS[EXPERIMENT_IDS.POST_COMPACTION_CONTEXT]; + const frontendValue = options?.experiments?.postCompactionContext; + + let postCompactionContextEnabled: boolean | undefined; + if (postCompactionExperiment.userOverridable && frontendValue !== undefined) { + // User-overridable: trust frontend value (user's explicit choice) + postCompactionContextEnabled = frontendValue; + } else if (this.experimentsService?.isRemoteEvaluationEnabled() === true) { + // Remote evaluation: use PostHog assignment + postCompactionContextEnabled = this.experimentsService.isExperimentEnabled( + EXPERIMENT_IDS.POST_COMPACTION_CONTEXT + ); + } else { + // Fallback to frontend value (dev mode or telemetry disabled) + postCompactionContextEnabled = frontendValue; + } + + const resolvedOptions = + postCompactionContextEnabled === undefined + ? options + : { + ...(options ?? { model: defaultModel }), + experiments: { + ...(options?.experiments ?? {}), + postCompactionContext: postCompactionContextEnabled, + }, + }; + + if (this.aiService.isStreaming(workspaceId) && !resolvedOptions?.editMessageId) { const pendingAskUserQuestion = askUserQuestionManager.getLatestPending(workspaceId); if (pendingAskUserQuestion) { try { @@ -1013,11 +1072,11 @@ export class WorkspaceService extends EventEmitter { } } - session.queueMessage(message, options); + session.queueMessage(message, resolvedOptions); return Ok(undefined); } - const result = await session.sendMessage(message, options); + const result = await session.sendMessage(message, resolvedOptions); if (!result.success) { log.error("sendMessage handler: session returned error", { workspaceId, diff --git a/tests/ipc/setup.ts b/tests/ipc/setup.ts index aed9cbbfbd..38eb3a1a08 100644 --- a/tests/ipc/setup.ts +++ b/tests/ipc/setup.ts @@ -84,6 +84,7 @@ export async function createTestEnvironment(): Promise { mcpServerManager: services.mcpServerManager, menuEventService: services.menuEventService, voiceService: services.voiceService, + experimentsService: services.experimentsService, telemetryService: services.telemetryService, sessionUsageService: services.sessionUsageService, };