From e120d6dfaa16f91df97667d1886219f820897a49 Mon Sep 17 00:00:00 2001 From: npub1223z34hd7vtwc6qj4s7flsxkj644nlre2nthu7lrrmkumhu3xddsrx9r6w <52a228d6edf316ec6812ac3c9fc0d696ab59fc7954d77e7be31eedcddf91335b@sprout-oss.stage.blox.sqprod.co> Date: Mon, 8 Jun 2026 16:54:57 -0700 Subject: [PATCH] refactor(features): preview-only manifest + screenshot CI wiring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per tho's clarification, the manifest is preview-only by design — a bad rebase had widened it to include stable entries with an explicit `tier` field. Restoring the original shape: - Rename `features.json` -> `preview-features.json` (loader, alias, ts-config, vite alias, test-loader-hooks, helpers, log strings). - Drop the 4 stable entries from the manifest. Only the 4 preview features remain (workflows, projects, pulse, forum). - Drop the `tier` field from the schema, types, and Zod validator — manifest membership is now sufficient ("in the file = preview; absent = stable, fail-open"). - Simplify `resolveEnabled(featureId, overrides)` — once you're inside it, the feature is preview by definition. - `useFeatureEnabled`: in-manifest -> check overrides; otherwise return true (fail-open). - `usePreviewFeatureWarning`: gate on manifest membership instead of `tier === 'preview'`. - Settings: `ExperimentalFeaturesCard` lists every desktop feature in the manifest directly; SettingsView feature gate uses the new `resolveEnabled` signature. - Tests: rewrote `resolveEnabled.test.mjs` for the new signature; helper drops the tier filter. Per Marge's review on the previous push, `screenshot-feature-flags.ts` was dropped from smoke testMatch but no CI step invoked the dedicated screenshot config — coverage was dark. Adding a `Desktop screenshot e2e` step in `.github/workflows/ci.yml` that runs `--config= playwright-screenshot.config.ts` after the smoke step, restoring coverage without dirtying smoke. Signed-off-by: npub1223z34hd7vtwc6qj4s7flsxkj644nlre2nthu7lrrmkumhu3xddsrx9r6w <52a228d6edf316ec6812ac3c9fc0d696ab59fc7954d77e7be31eedcddf91335b@sprout-oss.stage.blox.sqprod.co> --- .github/workflows/ci.yml | 2 + .../settings/ui/ExperimentalFeaturesCard.tsx | 4 +- .../src/features/settings/ui/SettingsView.tsx | 9 ++- desktop/src/shared/features/index.ts | 1 - desktop/src/shared/features/manifest.ts | 6 +- .../shared/features/resolveEnabled.test.mjs | 40 ++++-------- desktop/src/shared/features/resolveEnabled.ts | 23 +++---- desktop/src/shared/features/types.ts | 12 ++-- .../src/shared/features/useFeatureEnabled.ts | 31 +++++----- desktop/test-loader-hooks.mjs | 2 +- desktop/tests/helpers/bridge.ts | 2 +- desktop/tests/helpers/features.ts | 14 ++--- desktop/tsconfig.json | 2 +- desktop/vite.config.ts | 2 +- features.json | 61 ------------------- preview-features.json | 37 +++++++++++ 16 files changed, 101 insertions(+), 147 deletions(-) delete mode 100644 features.json create mode 100644 preview-features.json diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 597979f98..19bad7950 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -170,6 +170,8 @@ jobs: run: just desktop-build - name: Desktop smoke e2e run: cd desktop && pnpm exec playwright test --project=smoke + - name: Desktop screenshot e2e + run: cd desktop && pnpm exec playwright test --config=playwright-screenshot.config.ts - name: Desktop Tauri check run: just desktop-tauri-check env: diff --git a/desktop/src/features/settings/ui/ExperimentalFeaturesCard.tsx b/desktop/src/features/settings/ui/ExperimentalFeaturesCard.tsx index 63bdbed70..96ac8dedb 100644 --- a/desktop/src/features/settings/ui/ExperimentalFeaturesCard.tsx +++ b/desktop/src/features/settings/ui/ExperimentalFeaturesCard.tsx @@ -25,7 +25,9 @@ function FeatureRow({ feature }: { feature: FeatureDefinition }) { } export function ExperimentalFeaturesCard() { - const previewFeatures = desktopFeatures.filter((f) => f.tier === "preview"); + // Manifest is preview-only by definition; every desktop entry is a preview + // feature. + const previewFeatures = desktopFeatures; return (
diff --git a/desktop/src/features/settings/ui/SettingsView.tsx b/desktop/src/features/settings/ui/SettingsView.tsx index 2aa1ce70d..4bedcad3c 100644 --- a/desktop/src/features/settings/ui/SettingsView.tsx +++ b/desktop/src/features/settings/ui/SettingsView.tsx @@ -131,13 +131,12 @@ export function SettingsView({ const membership = myMembershipQuery.data; return settingsSections.filter((s) => { - // Feature gate check + // Feature gate check. Manifest is preview-only — if the gate id is in + // the manifest, it's preview and needs an opt-in; if it's not, it's + // stable and renders unconditionally (fail-open). if (s.featureGate) { const feature = getFeature(s.featureGate); - if ( - feature && - !resolveEnabled(feature.tier, feature.id, featureState) - ) { + if (feature && !resolveEnabled(s.featureGate, featureState)) { return false; } } diff --git a/desktop/src/shared/features/index.ts b/desktop/src/shared/features/index.ts index 380ac2708..9f6db1b15 100644 --- a/desktop/src/shared/features/index.ts +++ b/desktop/src/shared/features/index.ts @@ -5,7 +5,6 @@ export type { FeatureDefinition, FeaturesManifest, FeaturePlatform, - FeatureTier, } from "./types"; export { useFeatureEnabled, diff --git a/desktop/src/shared/features/manifest.ts b/desktop/src/shared/features/manifest.ts index 8797854c2..ac6d4d21a 100644 --- a/desktop/src/shared/features/manifest.ts +++ b/desktop/src/shared/features/manifest.ts @@ -3,20 +3,18 @@ import { z } from "zod"; import type { FeatureDefinition, FeaturesManifest } from "./types"; // --------------------------------------------------------------------------- -// Schema — runtime-validates the bundled features.json at startup. +// Schema — runtime-validates the bundled preview-features.json at startup. // // On parse failure we fall back to an empty manifest and log a console warning. // The app keeps working; gated UI stays hidden; nothing accidentally leaks. // --------------------------------------------------------------------------- -const FeatureTierSchema = z.enum(["stable", "preview"]); const FeaturePlatformSchema = z.enum(["desktop", "mobile"]); const FeatureDefinitionSchema = z.object({ id: z.string().min(1), name: z.string().min(1), description: z.string(), - tier: FeatureTierSchema, platforms: z.array(FeaturePlatformSchema).optional(), }); @@ -31,7 +29,7 @@ function loadManifest(): FeaturesManifest { const result = FeaturesManifestSchema.safeParse(manifestJson); if (!result.success) { console.warn( - "[FeatureFlags] features.json failed schema validation; falling back to empty manifest.", + "[FeatureFlags] preview-features.json failed schema validation; falling back to empty manifest.", result.error.issues, ); return EMPTY_MANIFEST; diff --git a/desktop/src/shared/features/resolveEnabled.test.mjs b/desktop/src/shared/features/resolveEnabled.test.mjs index 00ed0bbfe..69414249d 100644 --- a/desktop/src/shared/features/resolveEnabled.test.mjs +++ b/desktop/src/shared/features/resolveEnabled.test.mjs @@ -3,38 +3,20 @@ import { describe, it } from "node:test"; import { resolveEnabled } from "./resolveEnabled.ts"; -describe("resolveEnabled", () => { - describe("stable tier", () => { - it("always returns true regardless of overrides", () => { - assert.equal(resolveEnabled("stable", "doctor", {}), true); - assert.equal(resolveEnabled("stable", "doctor", { doctor: false }), true); - }); +describe("resolveEnabled (preview-only)", () => { + it("returns false by default (no override)", () => { + assert.equal(resolveEnabled("workflows", {}), false); }); - describe("preview tier", () => { - it("returns false by default (no override)", () => { - assert.equal(resolveEnabled("preview", "workflows", {}), false); - }); - - it("returns true when user opts in", () => { - assert.equal( - resolveEnabled("preview", "workflows", { workflows: true }), - true, - ); - }); - - it("returns false when user explicitly opts out", () => { - assert.equal( - resolveEnabled("preview", "workflows", { workflows: false }), - false, - ); - }); + it("returns true when user opts in", () => { + assert.equal(resolveEnabled("workflows", { workflows: true }), true); }); - describe("unknown tier", () => { - it("returns false for unrecognized tier values", () => { - // @ts-expect-error — testing invalid input - assert.equal(resolveEnabled("unknown", "foo", {}), false); - }); + it("returns false when user explicitly opts out", () => { + assert.equal(resolveEnabled("workflows", { workflows: false }), false); + }); + + it("ignores overrides for unrelated ids", () => { + assert.equal(resolveEnabled("workflows", { pulse: true }), false); }); }); diff --git a/desktop/src/shared/features/resolveEnabled.ts b/desktop/src/shared/features/resolveEnabled.ts index 27d344d78..1b0e4665b 100644 --- a/desktop/src/shared/features/resolveEnabled.ts +++ b/desktop/src/shared/features/resolveEnabled.ts @@ -1,20 +1,17 @@ -import type { FeatureTier } from "./types"; - /** - * Pure resolution logic for feature visibility. - * No side effects, no imports beyond types — safe to test in isolation. + * Pure resolution logic for preview-feature visibility. + * No side effects, no imports — 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. + * + * Returns true only if the user has explicitly opted in via overrides. */ export function resolveEnabled( - tier: FeatureTier, featureId: string, overrides: Record, ): boolean { - switch (tier) { - case "stable": - return true; - case "preview": - return overrides[featureId] === true; - default: - return false; - } + return overrides[featureId] === true; } diff --git a/desktop/src/shared/features/types.ts b/desktop/src/shared/features/types.ts index 9fa5e50a3..266bae6f4 100644 --- a/desktop/src/shared/features/types.ts +++ b/desktop/src/shared/features/types.ts @@ -1,15 +1,17 @@ -/** Feature visibility tiers */ -export type FeatureTier = "stable" | "preview"; - /** Platforms a feature is available on */ export type FeaturePlatform = "desktop" | "mobile"; -/** A single feature definition from the manifest */ +/** + * A single feature definition from the manifest. + * + * The manifest (`preview-features.json`) lists ONLY preview features — + * membership signals "this needs gating." Anything not in the manifest is + * treated as stable and renders unconditionally (fail-open). + */ export interface FeatureDefinition { id: string; name: string; description: string; - tier: FeatureTier; /** If omitted, feature is available on all platforms */ platforms?: FeaturePlatform[]; } diff --git a/desktop/src/shared/features/useFeatureEnabled.ts b/desktop/src/shared/features/useFeatureEnabled.ts index 53c633ca1..9253c482c 100644 --- a/desktop/src/shared/features/useFeatureEnabled.ts +++ b/desktop/src/shared/features/useFeatureEnabled.ts @@ -91,29 +91,24 @@ export function useFeatureSnapshot(): Record { } /** - * Returns whether a feature is enabled given its tier and user overrides. + * Returns whether a feature is enabled. * - * - stable: always true - * - preview: true only if user opted in - * - unknown id: fail-open (returns true). Manifest membership signals "this - * needs gating"; absence means "just render it." A stray `` - * pointing at a removed id should not hide UI. Dev mode still logs a - * `console.warn` so typos surface during development. + * The manifest (`preview-features.json`) lists ONLY preview features: + * + * - in manifest (preview): true only if the user opted in via overrides + * - NOT in manifest (stable): always true (fail-open) + * + * Membership in the manifest signals "this needs gating"; absence means + * "just render it." A stray `` will never + * hide UI. */ export function useFeatureEnabled(featureId: string): boolean { const overrides = useFeatureSnapshot(); const feature = getFeature(featureId); - if (!feature) { - if (import.meta.env.DEV) { - console.warn( - `[FeatureFlags] Unknown feature id: "${featureId}". Check features.json.`, - ); - } - return true; - } + if (!feature) return true; - return resolveEnabled(feature.tier, featureId, overrides); + return resolveEnabled(featureId, overrides); } /** @@ -153,7 +148,9 @@ export function usePreviewFeatureWarning(featureId: string): void { const feature = getFeature(featureId); useEffect(() => { - if (feature?.tier !== "preview" || enabled) return; + // No-op for stable features (not in manifest) and preview features + // that ARE enabled. Manifest membership = preview by definition. + if (!feature || enabled) return; let cancelled = false; void import("sonner").then(({ toast }) => { if (cancelled) return; diff --git a/desktop/test-loader-hooks.mjs b/desktop/test-loader-hooks.mjs index 1eee95364..f6540ee72 100644 --- a/desktop/test-loader-hooks.mjs +++ b/desktop/test-loader-hooks.mjs @@ -13,7 +13,7 @@ const repoRoot = path.resolve( export function resolve(specifier, context, nextResolve) { if (specifier === "@features-manifest") { - const resolved = path.join(repoRoot, "features.json"); + const resolved = path.join(repoRoot, "preview-features.json"); return nextResolve(resolved, context); } if (specifier.startsWith("@/")) { diff --git a/desktop/tests/helpers/bridge.ts b/desktop/tests/helpers/bridge.ts index 8e334acc9..8f14cc519 100644 --- a/desktop/tests/helpers/bridge.ts +++ b/desktop/tests/helpers/bridge.ts @@ -99,7 +99,7 @@ type BridgeOptions = { relayWsUrl?: string; skipOnboardingSeed?: boolean; /** - * When true (default), seed every preview feature in features.json as + * When true (default), seed every preview feature in preview-features.json as * enabled in localStorage so E2E tests can interact with gated UI without * clicking through the Experiments settings panel. Set to false in specs * that test the toggle behavior itself (e.g. diff --git a/desktop/tests/helpers/features.ts b/desktop/tests/helpers/features.ts index ade9759eb..fbcb4a7c9 100644 --- a/desktop/tests/helpers/features.ts +++ b/desktop/tests/helpers/features.ts @@ -1,15 +1,16 @@ // Single source of truth for E2E tests: derive the preview-feature list from -// /features.json so we don't have to hand-maintain a parallel array. +// /preview-features.json so we don't have to hand-maintain a parallel array. // -// Tier transitions (preview → stable, or new preview features added) are -// picked up automatically by every test that imports from here. -import featuresManifest from "../../../features.json" with { type: "json" }; +// New preview features added to the manifest are picked up automatically by +// every test that imports from here. +import featuresManifest from "../../../preview-features.json" with { + type: "json", +}; interface FeatureDefinition { id: string; name: string; description: string; - tier: "stable" | "preview"; platforms?: string[]; } @@ -20,8 +21,7 @@ interface FeaturesManifest { const manifest = featuresManifest as FeaturesManifest; -/** IDs of every preview-tier feature on desktop. */ +/** IDs of every preview feature on desktop. */ export const PREVIEW_FEATURE_IDS: string[] = manifest.features - .filter((f) => f.tier === "preview") .filter((f) => !f.platforms || f.platforms.includes("desktop")) .map((f) => f.id); diff --git a/desktop/tsconfig.json b/desktop/tsconfig.json index 9ca2a735e..e8f442a3e 100644 --- a/desktop/tsconfig.json +++ b/desktop/tsconfig.json @@ -7,7 +7,7 @@ "skipLibCheck": true, "paths": { "@/*": ["./src/*"], - "@features-manifest": ["../features.json"] + "@features-manifest": ["../preview-features.json"] }, /* Bundler mode */ diff --git a/desktop/vite.config.ts b/desktop/vite.config.ts index 1bac6c5bb..bea570482 100644 --- a/desktop/vite.config.ts +++ b/desktop/vite.config.ts @@ -25,7 +25,7 @@ export default defineConfig(async () => ({ resolve: { alias: { "@": "/src", - "@features-manifest": path.resolve(__dirname, "../features.json"), + "@features-manifest": path.resolve(__dirname, "../preview-features.json"), }, }, diff --git a/features.json b/features.json deleted file mode 100644 index 64c7c5922..000000000 --- a/features.json +++ /dev/null @@ -1,61 +0,0 @@ -{ - "version": 1, - "features": [ - { - "id": "managed-agents", - "name": "Managed Agents", - "description": "Create, configure, and run AI agents in your workspace", - "tier": "stable", - "platforms": ["desktop"] - }, - { - "id": "channel-templates", - "name": "Channel Templates", - "description": "Pre-configured channel setups with agents and workflows", - "tier": "stable", - "platforms": ["desktop"] - }, - { - "id": "custom-emoji", - "name": "Custom Emoji", - "description": "Workspace emoji palette for reactions and messages", - "tier": "stable", - "platforms": ["desktop"] - }, - { - "id": "doctor", - "name": "Doctor", - "description": "Diagnostic and debug panel for troubleshooting", - "tier": "stable", - "platforms": ["desktop"] - }, - { - "id": "workflows", - "name": "Workflows", - "description": "YAML-defined automations with approval gates", - "tier": "preview", - "platforms": ["desktop"] - }, - { - "id": "projects", - "name": "Projects", - "description": "Git repository browser and collaboration", - "tier": "preview", - "platforms": ["desktop"] - }, - { - "id": "pulse", - "name": "Pulse", - "description": "Activity feed with notes, social posts, and agent activity", - "tier": "preview", - "platforms": ["desktop"] - }, - { - "id": "forum", - "name": "Forum Channels", - "description": "Forum-style threaded channels for long-form discussions", - "tier": "preview", - "platforms": ["desktop"] - } - ] -} diff --git a/preview-features.json b/preview-features.json new file mode 100644 index 000000000..38ea181bb --- /dev/null +++ b/preview-features.json @@ -0,0 +1,37 @@ +{ + "version": 1, + "features": [ + { + "id": "workflows", + "name": "Workflows", + "description": "YAML-defined automations with approval gates", + "platforms": [ + "desktop" + ] + }, + { + "id": "projects", + "name": "Projects", + "description": "Git repository browser and collaboration", + "platforms": [ + "desktop" + ] + }, + { + "id": "pulse", + "name": "Pulse", + "description": "Activity feed with notes, social posts, and agent activity", + "platforms": [ + "desktop" + ] + }, + { + "id": "forum", + "name": "Forum Channels", + "description": "Forum-style threaded channels for long-form discussions", + "platforms": [ + "desktop" + ] + } + ] +}