diff --git a/.changeset/hyperdx-labs.md b/.changeset/hyperdx-labs.md new file mode 100644 index 0000000000..b2d59a4a88 --- /dev/null +++ b/.changeset/hyperdx-labs.md @@ -0,0 +1,13 @@ +--- +'@hyperdx/common-utils': minor +'@hyperdx/api': minor +'@hyperdx/app': minor +--- + +Add HyperDX Labs, a per-user opt-in for features that are still being built. +Open it from the user menu in the nav to see what's available and switch +individual experiments on or off. Everything is off by default, and your choices +are saved to your account rather than the browser, so they follow you across +devices (in local mode, where there is no account, they are kept in the browser). +Adds `PATCH /me/labs` and surfaces the opt-ins on `GET /me`. No experiments ship +in this release — this is the mechanism they'll use. diff --git a/AGENTS.md b/AGENTS.md index 24c1c24400..94c83253a8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -65,6 +65,9 @@ directory: actively coding) - `agent_docs/observability.md` - Instrumentation standards (tracing, metrics, context) and the shared helpers (read when adding or changing a feature) +- `agent_docs/labs.md` - HyperDX Labs: putting an unfinished feature behind a + per-user opt-in instead of a branch or a redeploy, and how to graduate or + retire it (read before adding a feature flag of any kind) **Package-specific guides** (read when working on that package): diff --git a/agent_docs/README.md b/agent_docs/README.md index 3a1f5444ea..c7093582ef 100644 --- a/agent_docs/README.md +++ b/agent_docs/README.md @@ -20,6 +20,7 @@ Instead of stuffing all instructions into `AGENTS.md` (which goes into every con - **`data_viz_colors.md`** - Chart, heatmap, and semantic status colors. Read before adding or changing any color in a chart, sparkline, heatmap, legend, or status pill. - **`themes.md`** - How the brand theme system (HyperDX vs ClickStack) and color mode (light/dark/system) work. Read before changing anything in `packages/app/src/theme/`, adding semantic CSS variables, or touching brand-conditional UI. - **`evals.md`** - MCP eval framework: dual-slot setup, running A/B comparisons between branches, interpreting results. Read before running evals or benchmarking MCP changes. +- **`labs.md`** - HyperDX Labs: how to put a half-finished feature behind a per-user opt-in, and how to graduate or retire it. Read before adding a feature flag of any kind. ## Usage Pattern diff --git a/agent_docs/labs.md b/agent_docs/labs.md new file mode 100644 index 0000000000..09a9d7897f --- /dev/null +++ b/agent_docs/labs.md @@ -0,0 +1,173 @@ +# HyperDX Labs + +Per-user, server-persisted opt-ins for features that aren't finished yet. + +The point is to stop choosing between a long-lived branch and a redeploy. Merge +the half-built thing to `main` behind a lab, let the people who want it turn it +on, and collect feedback while you finish. Off by default, so nobody is +surprised. + +**Labs are a user choice.** If the toggle is really a deployment decision — this +install doesn't have the backend, or the feature is off everywhere until launch — +it isn't a lab, it's a constant in `packages/app/src/config.ts` next to +`IS_MTVIEWS_ENABLED`. + +## Adding a lab + +Two edits. Add an entry to `packages/app/src/labs/registry.ts`: + +```ts +export const LABS: readonly Lab[] = [ + { + id: 'trace-flamegraph', + title: 'Trace flamegraph', + description: + 'Renders the trace waterfall as a flamegraph. Span links are not drawn yet, and very wide traces can be slow to lay out.', + badge: 'Alpha', + addedAt: '2026-08-14', + owner: '@your-handle', + }, +]; +``` + +Then gate the feature: + +```ts +import { useIsLabEnabled } from '@/labs/useLabs'; + +const isFlamegraphEnabled = useIsLabEnabled('trace-flamegraph'); +... +{isFlamegraphEnabled ? : } +``` + +No API change, no schema change, no migration, no new endpoint. The server +stores whatever ids the client sends (bounded — see below), so it never needs to +learn about your lab. + +Write the `description` for someone deciding whether to opt in. Say what's rough, +not just what's new: "span links aren't drawn yet" beats "improved trace view". + +### Checklist + +- `id` is kebab-case (`[a-z0-9]+(-[a-z0-9]+)*`) and permanent. It's persisted on + the user document, so renaming it silently resets everyone's opt-in. +- `addedAt` is today, `owner` is you. Both feed the graduate-or-retire sweep. +- Add an E2E test that toggles the lab, reloads, and asserts it stuck. + `packages/app/tests/e2e/components/LabsModalComponent.ts` has `setLab(id, on)` + (clicks and waits for the PATCH) and `labSwitch(id)` (the input, for + `toBeChecked()`) waiting for exactly this. Both were verified against a + throwaway entry, but the committed suite only covers the empty state, so yours + will be the first run in CI. +- A changeset, if the lab is visible to users at all. + +## Gating rules + +`useIsLabEnabled(id)` returns `false` when the user hasn't opted in and while the +stored set is still loading. Two consequences worth knowing: + +**The loading window is real** (outside local mode). `/me` isn't resolved on +first paint — `AuthLoadingBlocker` is only mounted on the landing page, so it +does not gate the app. A lab therefore reads OFF and then flips ON a moment +later. For the common shape (an extra tab, an extra button, an alternate +renderer) that's fine. If the flip is user-visible — a redirect, a default tab, a +one-shot effect, a mount-time fetch — use `useLabs()` and branch on `isLoading` +first: + +```ts +const { enabled, isLoading } = useLabs(); +if (isLoading) return ; +``` + +**Labs are available everywhere, including local mode**, so there is no +deployment flag to flip and no environment where the menu entry is missing. Local +mode has no API server and no user identity, so opt-ins there live in +localStorage under `hdx-labs` instead of on the user document. That branch is +confined to `useLabs.ts`; nothing else needs to know which store is in play. + +What local mode does *not* change: whether your feature can actually run. If it +needs a backend, gate on that too, the way `IS_IAC_EXPORT_ENABLED` does: + +```ts +const isEnabled = useIsLabEnabled('remote-mtviews') && !IS_LOCAL_MODE; +``` + +Now that a lab can be switched on in local mode, this is load-bearing rather +than decorative — without it, a local-mode user can opt into something that +cannot work. Add it whenever the feature depends on the API server. + +## Graduating and retiring + +**A lab is a commitment to decide, not a commitment to ship.** About 60 days +after `addedAt`, the owner picks one of two exits. "Leave it in Labs" is not an +exit — that's how you end up with fourteen flags nobody can explain. + +- **Graduate** — delete the registry entry, delete the gate, keep the new branch + of the conditional. +- **Retire** — delete the registry entry, delete the gate, delete the feature. + +Either way: **after deleting the entry, grep for the id.** A gate whose id has no +registry entry silently reads `false` forever, so nothing breaks loudly — the +gated code just goes dead and stays in the tree. The grep is the whole retirement +checklist. + +Stored `true` values for a deleted id go inert immediately (the hook derives +state from the registry, not from what's stored) and are pruned from the document +the next time that user toggles anything. No migration, no cleanup job. + +Enforcement is a review habit, not CI. A date-based test would fail on somebody +else's unrelated PR at 2am and get its constant bumped within the hour. The +registry is one short file — reading it is a 30-second sweep, and that +single-file-ness is the actual anti-rot mechanism. + +## How it works + +| Piece | Where | +| --- | --- | +| Registry (ids + UI copy) | `packages/app/src/labs/registry.ts` | +| Hook — the only seam | `packages/app/src/labs/useLabs.ts` | +| Modal (nav user menu) | `packages/app/src/labs/LabsModal.tsx` | +| Storage | `labs` on the `User` document, `packages/api/src/models/user.ts` | +| Read / write | `GET /me` and `PATCH /me/labs`, `packages/api/src/routers/api/me.ts` | +| Local-mode storage | `hdx-labs` in localStorage, via the atom in `useLabs.ts` | +| Shape contract | `UserLabsSchema` in `packages/common-utils/src/types.ts` | + +There is no deployment flag. Labs is always present; individual labs are what +get toggled. + +State is an **enabled-set**: a key present with `true` is on, an absent key is +off. Writes are **full replace** — the client always holds the whole registry, so +it can always compute the complete desired set, and that's what makes retired ids +self-pruning. + +**The server validates shape, not ids.** It bounds the key format (kebab-case, +which also excludes `$`, `.` and `_`, so Mongo operators, dotted paths and +`__proto__` can't be keys) and the entry count, but it deliberately does not know +which labs exist — that's what keeps adding one to a single file. The trade-off: +a typo'd id can't be rejected server-side, so +`packages/app/src/labs/__tests__/registry.test.ts` parses every registry id +against `LabIdSchema` to catch `my_lab` or `My-Lab` at CI time instead of as a +mystery 400. + +Reads compare `=== true` rather than truthiness, because a key like `constructor` +passes the id regex and is inherited from `Object.prototype`, where it's truthy. + +**Multi-tab / multi-device:** a toggle is visible immediately in the tab you +clicked it in (the mutation is optimistic), in other tabs on next focus, and on +other devices on next load. In local mode there is nothing to sync: the choice +stays in that browser. + +Because a full replacement is only safe if the newest one lands last, the +mutation carries `scope: { id: 'user-labs' }`, which makes react-query send +toggles one at a time in call order. Flipping two switches quickly is therefore +safe within a tab. Across two tabs or two devices it is still last-write-wins — +they are separate clients with separate queues, so the loser has to flip again. +If that ever stops being acceptable, merge server-side in `setUserLabs`. + +## Related + +`packages/app/src/hooks/useIsVariablesEnabled.ts` predates this and is the +clearest example of what labs are for: an env-var toggle +(`NEXT_PUBLIC_ENABLE_DASHBOARD_VARIABLES`) shipped because variable substitution +wasn't implemented yet, wrapped in a hook whose `isLoading: false` was left in +place, per its own comment, "to support team-level toggle loading in the future." +That's the shape `useLabs` now provides — a good first graduation candidate. diff --git a/packages/api/src/controllers/user.ts b/packages/api/src/controllers/user.ts index 55258c2db6..61a4b4a0b7 100644 --- a/packages/api/src/controllers/user.ts +++ b/packages/api/src/controllers/user.ts @@ -1,3 +1,4 @@ +import type { UserLabs } from '@hyperdx/common-utils/dist/types'; import mongoose from 'mongoose'; import type { ObjectId } from '@/models'; @@ -20,6 +21,28 @@ export function findUsersByTeam(team: string | ObjectId) { return User.find({ team }).sort({ createdAt: 1 }); } +/** + * Replaces a user's lab opt-ins wholesale. + * + * Whole-object `$set`, deliberately not `$set: { ['labs.' + id]: value }`: a + * dotted path is the one place a client-supplied key stops being update *data* + * and becomes part of the update *instruction*. Keeping keys in value position + * removes that class of bug outright, which is what lets LabIdSchema's key + * regex be defense-in-depth rather than the only defense. + * + * There is nothing to read first because the semantics are full-replace: the + * read-modify-write happens on the client, which is the only place that knows + * the current lab registry and therefore the only place that can prune the ids + * of retired labs. See agent_docs/labs.md. + */ +export function setUserLabs(userId: ObjectId, labs: UserLabs) { + return User.findByIdAndUpdate( + userId, + { $set: { labs } }, + { new: true, projection: { labs: 1 } }, + ); +} + export async function deleteTeamMember( teamId: string | ObjectId, userIdToDelete: string, diff --git a/packages/api/src/models/user.ts b/packages/api/src/models/user.ts index 8f60fadeb5..6e0513d51a 100644 --- a/packages/api/src/models/user.ts +++ b/packages/api/src/models/user.ts @@ -12,6 +12,17 @@ export interface IUser { email: string; name: string; team: ObjectId; + /** + * Per-user opt-ins for in-development features ("HyperDX Labs"). An + * enabled-set: a key present with `true` is on, an absent key is off. + * + * Deliberately not typed per-lab. The registry lives in + * packages/app/src/labs/registry.ts so that adding a lab never has to touch + * this file; ids and count are bounded on the write path by UserLabsSchema. + * Absent on every document created before labs existed, which reads as "no + * labs enabled". See agent_docs/labs.md. + */ + labs?: Record; } export type UserDocument = mongoose.HydratedDocument; @@ -30,6 +41,13 @@ const UserSchema = new Schema( return uuidv4(); }, }, + // Mixed, not Map: a Mongoose Map's own toJSON() returns a *native* Map + // unless handed { flattenMaps: true }, and `GET /me` passes this value + // straight to res.json() — it would serialize as `{}` forever. See the + // MongooseMap note in packages/api/src/models/webhook.ts. Mixed round-trips + // as a plain object. Its one weakness (no change tracking on nested paths) + // never comes up because setUserLabs only ever $sets the whole object. + labs: { type: Schema.Types.Mixed }, }, { timestamps: true, diff --git a/packages/api/src/routers/api/__tests__/me.int.test.ts b/packages/api/src/routers/api/__tests__/me.int.test.ts new file mode 100644 index 0000000000..23bec4122d --- /dev/null +++ b/packages/api/src/routers/api/__tests__/me.int.test.ts @@ -0,0 +1,168 @@ +import { getAgent, getLoggedInAgent, getServer } from '@/fixtures'; +import User from '@/models/user'; + +describe('me router', () => { + const server = getServer(); + + beforeAll(async () => { + await server.start(); + }); + + afterEach(async () => { + await server.clearDBs(); + }); + + afterAll(async () => { + await server.stop(); + }); + + describe('GET /me', () => { + it('returns an empty lab set for a user that has never opted in', async () => { + const { agent } = await getLoggedInAgent(server); + + const resp = await agent.get('/me').expect(200); + + // `labs` is absent on the document; the route normalizes it so the client + // never has to distinguish "no field" from "nothing enabled". + expect(resp.body.labs).toEqual({}); + }); + }); + + describe('PATCH /me/labs', () => { + it('persists an opt-in and reflects it on GET /me', async () => { + const { agent } = await getLoggedInAgent(server); + + const patchResp = await agent + .patch('/me/labs') + .send({ labs: { 'my-lab': true } }) + .expect(200); + + expect(patchResp.body).toEqual({ labs: { 'my-lab': true } }); + + const meResp = await agent.get('/me').expect(200); + expect(meResp.body.labs).toEqual({ 'my-lab': true }); + }); + + it('serializes labs as a plain object', async () => { + // Regression guard for the storage type. A Mongoose Map's toJSON() + // returns a native Map unless given { flattenMaps: true }, which + // JSON.stringify renders as `{}` — so a future switch away from Mixed + // would silently start returning nothing. See models/webhook.ts. + const { agent } = await getLoggedInAgent(server); + + await agent + .patch('/me/labs') + .send({ labs: { 'lab-one': true, 'lab-two': true } }) + .expect(200); + + const resp = await agent.get('/me').expect(200); + + expect(typeof resp.body.labs).toBe('object'); + expect(Array.isArray(resp.body.labs)).toBe(false); + expect(resp.body.labs).toEqual({ 'lab-one': true, 'lab-two': true }); + expect(Object.keys(resp.body.labs).sort()).toEqual([ + 'lab-one', + 'lab-two', + ]); + }); + + it('replaces the whole set rather than merging', async () => { + // Full-replace is what makes ids from retired labs self-pruning. + const { agent } = await getLoggedInAgent(server); + + await agent + .patch('/me/labs') + .send({ labs: { 'lab-a': true, 'lab-b': true } }) + .expect(200); + + await agent + .patch('/me/labs') + .send({ labs: { 'lab-a': true } }) + .expect(200); + + const resp = await agent.get('/me').expect(200); + expect(resp.body.labs).toEqual({ 'lab-a': true }); + }); + + it.each([ + ['an underscore', { my_lab: true }], + ['uppercase', { 'My-Lab': true }], + ['a dotted path', { 'a.b': true }], + ['a mongo operator', { $set: true }], + ['a leading dash', { '-lab': true }], + ['an empty id', { '': true }], + ])('rejects a lab id containing %s', async (_label, labs) => { + const { agent } = await getLoggedInAgent(server); + + await agent.patch('/me/labs').send({ labs }).expect(400); + }); + + it('rejects __proto__ as a lab id without polluting Object.prototype', async () => { + const { agent } = await getLoggedInAgent(server); + + // Sent as a raw JSON string so `__proto__` arrives as an own property, + // which is how express.json() parses it off the wire. + await agent + .patch('/me/labs') + .set('Content-Type', 'application/json') + .send('{"labs":{"__proto__":true}}') + .expect(400); + + expect(Object.getPrototypeOf({})).toBe(Object.prototype); + expect({}).not.toHaveProperty('labs'); + }); + + it('rejects a non-boolean value', async () => { + const { agent } = await getLoggedInAgent(server); + + await agent + .patch('/me/labs') + .send({ labs: { 'lab-a': 'true' } }) + .expect(400); + }); + + it('rejects more lab entries than a user may store', async () => { + const { agent } = await getLoggedInAgent(server); + + const tooMany: Record = {}; + for (let i = 0; i < 65; i++) { + tooMany[`lab-${i}`] = true; + } + + await agent.patch('/me/labs').send({ labs: tooMany }).expect(400); + }); + + it('rejects an unauthenticated request', async () => { + const anon = getAgent(server); + + await anon + .patch('/me/labs') + .send({ labs: { 'lab-a': true } }) + .expect(401); + }); + + it('scopes the write to the requesting user, not the team', async () => { + // The second user is created directly rather than via getLoggedInAgent: + // POST /register/password refuses once a team exists, so a second session + // would need the invite flow. The concern here is only that the $set + // targets req.user._id, which this covers. + const { agent, team, user: userA } = await getLoggedInAgent(server); + const userB = await User.create({ + email: 'second@example.com', + name: 'Second', + team: team._id, + }); + + await agent + .patch('/me/labs') + .send({ labs: { 'lab-a': true } }) + .expect(200); + + const storedA = await User.findById(userA._id); + expect(storedA?.labs).toEqual({ 'lab-a': true }); + + const storedB = await User.findById(userB._id); + expect(storedB?.labs).toBeUndefined(); + }); + }); +}); diff --git a/packages/api/src/routers/api/me.ts b/packages/api/src/routers/api/me.ts index b574928528..65d55df698 100644 --- a/packages/api/src/routers/api/me.ts +++ b/packages/api/src/routers/api/me.ts @@ -1,9 +1,17 @@ -import type { MeApiResponse } from '@hyperdx/common-utils/dist/types'; +import type { + MeApiResponse, + UpdateUserLabsApiResponse, +} from '@hyperdx/common-utils/dist/types'; +import { UpdateUserLabsRequestSchema } from '@hyperdx/common-utils/dist/types'; import express from 'express'; +import { processRequest } from 'zod-express-middleware'; import { AI_API_KEY, ANTHROPIC_API_KEY, USAGE_STATS_ENABLED } from '@/config'; import { getTeam } from '@/controllers/team'; +import { setUserLabs } from '@/controllers/user'; +import { getNonNullUserWithTeam } from '@/middleware/auth'; import { Api404Error } from '@/utils/errors'; +import { setBusinessContext } from '@/utils/instrumentation'; import { sendJson } from '@/utils/serialization'; const router = express.Router(); @@ -19,6 +27,7 @@ router.get('/', async (req, res: express.Response, next) => { accessKey, createdAt, email, + labs, name, team: teamId, } = req.user; @@ -37,10 +46,46 @@ router.get('/', async (req, res: express.Response, next) => { team, usageStatsEnabled: USAGE_STATS_ENABLED, aiAssistantEnabled: !!(AI_API_KEY || ANTHROPIC_API_KEY), + // Absent on every user created before labs existed; `{}` reads as + // "nothing enabled", which is what the client expects. + labs: labs ?? {}, }); } catch (e) { next(e); } }); +/** + * Replaces the caller's lab opt-ins. A sub-path rather than `PATCH /me` because + * `GET /me` also returns email, name, accessKey and team — an endpoint that + * merely *looks* like it accepts edits to those is a hazard worth avoiding. + */ +router.patch( + '/labs', + processRequest({ body: UpdateUserLabsRequestSchema }), + async (req, res: express.Response, next) => { + try { + const { userId } = getNonNullUserWithTeam(req); + const { labs } = req.body; + + const user = await setUserLabs(userId, labs); + if (user == null) { + throw new Api404Error(`User not found for id ${userId}`); + } + + // A wide-event span attribute, deliberately not a metric: lab ids are + // client-supplied, so using them as metric attribute values would be + // unbounded cardinality. Team/user context is already on the span from + // isUserAuthenticated. See agent_docs/observability.md. + setBusinessContext({ + 'hyperdx.user.labs.enabled': Object.keys(labs).filter(id => labs[id]), + }); + + return sendJson(res, { labs: user.labs ?? {} }); + } catch (e) { + next(e); + } + }, +); + export default router; diff --git a/packages/app/src/api.ts b/packages/app/src/api.ts index 21facd9c05..23a826cd3a 100644 --- a/packages/app/src/api.ts +++ b/packages/app/src/api.ts @@ -76,6 +76,12 @@ export const hdxServer = ( }); }; +/** + * Query key for `useMe`. Exported so code that writes into the `/me` cache + * (src/labs/useLabs.ts) can't drift from the key `useMe` reads. + */ +export const ME_QUERY_KEY = ['me'] as const; + const api = { useCreateAlert() { return useMutation<{ data: Alert }, Error, Alert>({ @@ -291,7 +297,7 @@ const api = { }, useMe() { return useQuery({ - queryKey: [`me`], + queryKey: ME_QUERY_KEY, queryFn: () => { if (IS_LOCAL_MODE) { return null; diff --git a/packages/app/src/components/AppNav/AppNav.components.tsx b/packages/app/src/components/AppNav/AppNav.components.tsx index 95344a749c..553b098f75 100644 --- a/packages/app/src/components/AppNav/AppNav.components.tsx +++ b/packages/app/src/components/AppNav/AppNav.components.tsx @@ -20,6 +20,7 @@ import { IconChevronDown, IconChevronRight, IconChevronUp, + IconFlask, IconHelp, IconKeyboard, IconLogout, @@ -68,6 +69,7 @@ type AppNavUserMenuProps = { teamName?: string; logoutUrl?: string | null; onClickUserPreferences?: () => void; + onClickLabs?: () => void; }; const getUserInitials = (userName: string) => { @@ -85,6 +87,7 @@ export const AppNavUserMenu = ({ teamName, logoutUrl, onClickUserPreferences, + onClickLabs, }: AppNavUserMenuProps) => { const { isCollapsed } = React.useContext(AppNavContext); const resolvedUserName = userName.trim() || 'User'; @@ -161,6 +164,13 @@ export const AppNavUserMenu = ({ > User Preferences + } + onClick={onClickLabs} + > + HyperDX Labs + {logoutUrl && ( <> diff --git a/packages/app/src/components/AppNav/AppNav.tsx b/packages/app/src/components/AppNav/AppNav.tsx index 13dc89e695..0d794fd116 100644 --- a/packages/app/src/components/AppNav/AppNav.tsx +++ b/packages/app/src/components/AppNav/AppNav.tsx @@ -34,6 +34,7 @@ import { IS_LOCAL_MODE } from '@/config'; import { Dashboard, useDashboards } from '@/dashboard'; import { useFavorites } from '@/favorites'; import InstallInstructionModal from '@/InstallInstructionsModal'; +import { LabsModal } from '@/labs/LabsModal'; import OnboardingChecklist from '@/OnboardingChecklist'; import { useSavedSearches } from '@/savedSearch'; import { useLogomark, useWordmark } from '@/theme/ThemeProvider'; @@ -269,6 +270,8 @@ export default function AppNav({ fixed = false }: { fixed?: boolean }) { { close: closeUserPreferences, open: openUserPreferences }, ] = useDisclosure(false); + const [labsOpen, { close: closeLabs, open: openLabs }] = useDisclosure(false); + const { userPreferences: { isUTC }, } = useUserPreferences(); @@ -518,6 +521,7 @@ export default function AppNav({ fixed = false }: { fixed?: boolean }) { userName={meData?.name} teamName={meData?.team?.name} onClickUserPreferences={openUserPreferences} + onClickLabs={openLabs} logoutUrl={IS_LOCAL_MODE ? null : `/api/logout`} /> {meData?.usageStatsEnabled && ( @@ -539,6 +543,7 @@ export default function AppNav({ fixed = false }: { fixed?: boolean }) { opened={UserPreferencesOpen} onClose={closeUserPreferences} /> + ); } diff --git a/packages/app/src/labs/LabsModal.tsx b/packages/app/src/labs/LabsModal.tsx new file mode 100644 index 0000000000..fe933790a4 --- /dev/null +++ b/packages/app/src/labs/LabsModal.tsx @@ -0,0 +1,119 @@ +import * as React from 'react'; +import { Badge, Card, Group, Modal, Stack, Switch, Text } from '@mantine/core'; +import { IconFlask } from '@tabler/icons-react'; + +import type { Lab } from '@/labs/registry'; +import { LABS } from '@/labs/registry'; +import { useLabs } from '@/labs/useLabs'; + +const LabCard = ({ + lab, + enabled, + disabled, + onChange, +}: { + lab: Lab; + enabled: boolean; + disabled: boolean; + onChange: (enabled: boolean) => void; +}) => { + return ( + + +
+ + + {lab.title} + + {!!lab.badge && ( + + {lab.badge} + + )} + + + {lab.description} + +
+ {/* + Two testids on purpose. Mantine puts arbitrary props on the , + which it hides behind an aria-hidden track — so that one is assertable + (toBeChecked) but not clickable. wrapperProps lands on the enclosing + + + ); +}; + +export const LabsModal = ({ + opened, + onClose, +}: { + opened: boolean; + onClose: () => void; +}) => { + const { enabled, isLoading, setLabEnabled } = useLabs(); + + return ( + + +
+ HyperDX Labs + + Try features that are still being built + +
+ + } + size="lg" + padding="lg" + keepMounted={false} + opened={opened} + onClose={onClose} + > + {/* + Inner content carries its own testid: a Mantine Modal's root stays in the + DOM with zero dimensions, so it never reads as "visible" to Playwright. + Open-state assertions have to target content, closed-state the root. + */} + + {LABS.length === 0 ? ( + + No experiments are available right now. Check back soon. + + ) : ( + <> + + These are unfinished, so expect rough edges. Everything here is + off by default and you can turn it off again at any time. Your + choices are saved to your account, so they follow you across + browsers and devices. + + {LABS.map(lab => ( + setLabEnabled(lab.id, value)} + /> + ))} + + )} + + + ); +}; diff --git a/packages/app/src/labs/__tests__/registry.test.ts b/packages/app/src/labs/__tests__/registry.test.ts new file mode 100644 index 0000000000..1013c34cbb --- /dev/null +++ b/packages/app/src/labs/__tests__/registry.test.ts @@ -0,0 +1,42 @@ +import { LabIdSchema, LABS_MAX_KEYS } from '@hyperdx/common-utils/dist/types'; + +import { LABS } from '@/labs/registry'; + +/** + * The server bounds the *shape* of what a user can store but deliberately does + * not know the lab id list, so it cannot reject a typo'd id — see the Labs + * comment in common-utils/src/types.ts. These assertions are what catch + * `my_lab` or `My-Lab` at CI time instead of as a mystery 400 in the browser. + * + * They pass trivially while the registry is empty and start earning their keep + * with the first lab. + */ +describe('labs registry', () => { + it('gives every lab an id the API will accept', () => { + for (const lab of LABS) { + const result = LabIdSchema.safeParse(lab.id); + expect(result.success).toBe(true); + } + }); + + it('has no duplicate ids', () => { + const ids = LABS.map(lab => lab.id); + expect(new Set(ids).size).toBe(ids.length); + }); + + it('stays within the per-user storage cap', () => { + // Every lab in the registry can be enabled at once, so the registry itself + // must fit inside what UserLabsSchema will accept. + expect(LABS.length).toBeLessThanOrEqual(LABS_MAX_KEYS); + }); + + it('describes every lab well enough to opt into', () => { + for (const lab of LABS) { + expect(lab.title.trim()).not.toBe(''); + expect(lab.description.trim()).not.toBe(''); + expect(lab.owner.trim()).not.toBe(''); + // addedAt drives the graduate-or-retire sweep; see agent_docs/labs.md. + expect(lab.addedAt).toMatch(/^\d{4}-\d{2}-\d{2}$/); + } + }); +}); diff --git a/packages/app/src/labs/__tests__/useLabs.localMode.test.tsx b/packages/app/src/labs/__tests__/useLabs.localMode.test.tsx new file mode 100644 index 0000000000..d7076520ce --- /dev/null +++ b/packages/app/src/labs/__tests__/useLabs.localMode.test.tsx @@ -0,0 +1,127 @@ +import React from 'react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { act, renderHook, waitFor } from '@testing-library/react'; + +import { hdxServer } from '@/api'; +import { useLabs } from '@/labs/useLabs'; + +// Separate file because jest.mock is file-scoped and local mode is the only +// case that takes the localStorage path. +jest.mock('@/config', () => ({ IS_LOCAL_MODE: true })); + +// Mirrors real local mode, where useMe()'s queryFn short-circuits to null. +jest.mock('@/api', () => { + const { useQuery } = jest.requireActual('@tanstack/react-query'); + return { + __esModule: true, + ME_QUERY_KEY: ['me'], + hdxServer: jest.fn(), + default: { + useMe: () => useQuery({ queryKey: ['me'], queryFn: () => null }), + }, + }; +}); + +jest.mock('@/labs/registry', () => ({ + LABS: [ + { + id: 'lab-a', + title: 'Lab A', + description: 'A', + addedAt: '2026-01-01', + owner: '@test', + }, + { + id: 'lab-b', + title: 'Lab B', + description: 'B', + addedAt: '2026-01-01', + owner: '@test', + }, + ], +})); + +jest.mock('@mantine/notifications', () => ({ + notifications: { show: jest.fn() }, +})); + +const STORAGE_KEY = 'hdx-labs'; +const mockHdxServer = jest.mocked(hdxServer); + +function renderLabs() { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + return renderHook(() => useLabs(), { + wrapper: ({ children }) => ( + {children} + ), + }); +} + +beforeEach(() => { + jest.clearAllMocks(); + localStorage.clear(); +}); + +describe('useLabs in local mode', () => { + it('never reports loading, since localStorage is synchronous', () => { + const { result } = renderLabs(); + + expect(result.current.isLoading).toBe(false); + expect(result.current.isSaving).toBe(false); + expect(result.current.enabled).toEqual({ 'lab-a': false, 'lab-b': false }); + }); + + it('reads opt-ins from localStorage', async () => { + localStorage.setItem(STORAGE_KEY, JSON.stringify({ 'lab-a': true })); + + const { result } = renderLabs(); + + await waitFor(() => expect(result.current.enabled['lab-a']).toBe(true)); + expect(result.current.enabled['lab-b']).toBe(false); + }); + + it('persists a toggle to localStorage without calling the API', async () => { + const { result } = renderLabs(); + + act(() => result.current.setLabEnabled('lab-b', true)); + + await waitFor(() => expect(result.current.enabled['lab-b']).toBe(true)); + expect(JSON.parse(localStorage.getItem(STORAGE_KEY) ?? '{}')).toEqual({ + 'lab-b': true, + }); + // There is no API server in local mode; a request here would 404. + expect(mockHdxServer).not.toHaveBeenCalled(); + }); + + it('prunes ids that are no longer in the registry on write', async () => { + localStorage.setItem( + STORAGE_KEY, + JSON.stringify({ 'lab-a': true, 'retired-lab': true }), + ); + + const { result } = renderLabs(); + await waitFor(() => expect(result.current.enabled['lab-a']).toBe(true)); + + act(() => result.current.setLabEnabled('lab-b', true)); + + await waitFor(() => expect(result.current.enabled['lab-b']).toBe(true)); + expect(JSON.parse(localStorage.getItem(STORAGE_KEY) ?? '{}')).toEqual({ + 'lab-a': true, + 'lab-b': true, + }); + }); + + it('clears the entry when a lab is switched off', async () => { + localStorage.setItem(STORAGE_KEY, JSON.stringify({ 'lab-a': true })); + + const { result } = renderLabs(); + await waitFor(() => expect(result.current.enabled['lab-a']).toBe(true)); + + act(() => result.current.setLabEnabled('lab-a', false)); + + await waitFor(() => expect(result.current.enabled['lab-a']).toBe(false)); + expect(JSON.parse(localStorage.getItem(STORAGE_KEY) ?? '{}')).toEqual({}); + }); +}); diff --git a/packages/app/src/labs/__tests__/useLabs.test.tsx b/packages/app/src/labs/__tests__/useLabs.test.tsx new file mode 100644 index 0000000000..bc62edd536 --- /dev/null +++ b/packages/app/src/labs/__tests__/useLabs.test.tsx @@ -0,0 +1,330 @@ +import React from 'react'; +import type { MeApiResponse } from '@hyperdx/common-utils/dist/types'; +import { notifications } from '@mantine/notifications'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { act, renderHook, waitFor } from '@testing-library/react'; + +import { useIsLabEnabled, useLabs } from '@/labs/useLabs'; + +/** + * The mocked `hdxServer`, typed as what it actually is rather than as ky's + * `ResponsePromise`. Reaching for the real signature would mean asserting a + * bare `{ json }` stub into a generic `json()` interface, and the honest + * narrow type here avoids that cast entirely. The hook only ever calls + * `.json()` on the result. + */ +type HdxServerMock = jest.Mock< + { json: () => Promise }, + [string, { method?: string; json?: unknown }?] +>; + +// A never-resolving /me keeps the query pending, which is how the OFF -> ON +// window is exercised. +const PENDING = Symbol('pending'); + +let meFixture: Partial | null | typeof PENDING = null; + +// `useMe` is mocked as a *real* useQuery over the same key rather than as a +// static return value. The optimistic update writes straight into the +// react-query cache, so the cache has to be the actual source of truth or the +// optimism and rollback assertions would be testing nothing. +jest.mock('@/api', () => { + const { useQuery } = jest.requireActual('@tanstack/react-query'); + return { + __esModule: true, + ME_QUERY_KEY: ['me'], + hdxServer: jest.fn(), + default: { + useMe: () => + useQuery({ + queryKey: ['me'], + queryFn: () => + meFixture === PENDING + ? new Promise(() => { + /* never resolves */ + }) + : meFixture, + }), + }, + }; +}); + +// Explicit rather than relying on the env default: which store useLabs reads +// now hinges on this, and local mode is covered in useLabs.localMode.test.tsx. +jest.mock('@/config', () => ({ IS_LOCAL_MODE: false })); + +jest.mock('@/labs/registry', () => ({ + LABS: [ + { + id: 'lab-a', + title: 'Lab A', + description: 'A', + addedAt: '2026-01-01', + owner: '@test', + }, + { + id: 'lab-b', + title: 'Lab B', + description: 'B', + addedAt: '2026-01-01', + owner: '@test', + }, + ], +})); + +jest.mock('@mantine/notifications', () => ({ + notifications: { show: jest.fn() }, +})); + +const { hdxServer: mockHdxServer } = jest.requireMock<{ + hdxServer: HdxServerMock; +}>('@/api'); +const mockShow = jest.mocked(notifications.show); + +function resolvePatch(body: unknown = { labs: {} }) { + mockHdxServer.mockReturnValue({ json: () => Promise.resolve(body) }); +} + +/** Returns a `resolve`/`reject` pair so a test can hold the PATCH in flight. */ +function deferPatch() { + let settle: { resolve: () => void; reject: () => void }; + const promise = new Promise((res, rej) => { + settle = { + resolve: () => res({ labs: {} }), + reject: () => rej(new Error('nope')), + }; + }); + mockHdxServer.mockReturnValue({ json: () => promise }); + + return settle!; +} + +function renderLabs() { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + return renderHook(() => useLabs(), { + wrapper: ({ children }) => ( + {children} + ), + }); +} + +/** The PATCH body the hook sent, as an object. */ +function lastPatchBody() { + const call = mockHdxServer.mock.calls.at(-1); + return call?.[1]?.json; +} + +beforeEach(() => { + jest.clearAllMocks(); + meFixture = null; + resolvePatch(); +}); + +describe('useLabs', () => { + it('reports loading and every lab off while /me is pending', async () => { + meFixture = PENDING; + + const { result } = renderLabs(); + + expect(result.current.isLoading).toBe(true); + expect(result.current.enabled).toEqual({ + 'lab-a': false, + 'lab-b': false, + }); + }); + + it('reads every lab as off when /me carries no labs field', async () => { + // The pre-existing-user path: the field is absent on documents created + // before labs existed. + meFixture = { name: 'Test' }; + + const { result } = renderLabs(); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + expect(result.current.enabled).toEqual({ + 'lab-a': false, + 'lab-b': false, + }); + }); + + it('reflects stored opt-ins', async () => { + meFixture = { labs: { 'lab-a': true } }; + + const { result } = renderLabs(); + + await waitFor(() => expect(result.current.enabled['lab-a']).toBe(true)); + expect(result.current.enabled['lab-b']).toBe(false); + }); + + it('ignores stored ids that are not in the registry', async () => { + // A graduated or retired lab. It must not reach a consumer. + meFixture = { labs: { 'retired-lab': true, 'lab-a': true } }; + + const { result } = renderLabs(); + + await waitFor(() => expect(result.current.enabled['lab-a']).toBe(true)); + expect(result.current.enabled).not.toHaveProperty('retired-lab'); + expect(Object.keys(result.current.enabled).sort()).toEqual([ + 'lab-a', + 'lab-b', + ]); + }); + + it('does not treat a non-boolean stored value as enabled', async () => { + // Guards the `=== true` comparison rather than truthiness. + meFixture = { + labs: { 'lab-a': 'true' } as unknown as { 'lab-a': boolean }, + }; + + const { result } = renderLabs(); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + expect(result.current.enabled['lab-a']).toBe(false); + }); + + it('sends a full-replace payload that prunes unknown ids and omits disabled labs', async () => { + meFixture = { labs: { 'lab-a': true, 'retired-lab': true } }; + + const { result } = renderLabs(); + await waitFor(() => expect(result.current.enabled['lab-a']).toBe(true)); + + act(() => result.current.setLabEnabled('lab-b', true)); + + await waitFor(() => expect(mockHdxServer).toHaveBeenCalled()); + expect(mockHdxServer).toHaveBeenCalledWith( + 'me/labs', + expect.objectContaining({ method: 'PATCH' }), + ); + // retired-lab is gone, and nothing is stored as `false`. + expect(lastPatchBody()).toEqual({ + labs: { 'lab-a': true, 'lab-b': true }, + }); + }); + + it('sends an empty set when the last enabled lab is switched off', async () => { + meFixture = { labs: { 'lab-a': true } }; + + const { result } = renderLabs(); + await waitFor(() => expect(result.current.enabled['lab-a']).toBe(true)); + + act(() => result.current.setLabEnabled('lab-a', false)); + + await waitFor(() => expect(mockHdxServer).toHaveBeenCalled()); + expect(lastPatchBody()).toEqual({ labs: {} }); + }); + + it('applies the toggle optimistically before the request settles', async () => { + meFixture = { labs: {} }; + const settle = deferPatch(); + + const { result } = renderLabs(); + await waitFor(() => expect(result.current.isLoading).toBe(false)); + + act(() => result.current.setLabEnabled('lab-b', true)); + + // Still in flight, but the UI already shows the new state. + await waitFor(() => expect(result.current.enabled['lab-b']).toBe(true)); + expect(result.current.isSaving).toBe(true); + + await act(async () => { + settle.resolve(); + }); + }); + + it('serializes overlapping toggles so an older payload cannot land last', async () => { + // Without a mutation scope both requests fly concurrently, and if the first + // one's response lands last the server keeps its payload — dropping the + // second lab, which the next /me refetch then reverts in the UI. + meFixture = { labs: {} }; + const first = deferPatch(); + + const { result } = renderLabs(); + await waitFor(() => expect(result.current.isLoading).toBe(false)); + + act(() => result.current.setLabEnabled('lab-a', true)); + await waitFor(() => expect(result.current.enabled['lab-a']).toBe(true)); + + act(() => result.current.setLabEnabled('lab-b', true)); + await waitFor(() => expect(result.current.enabled['lab-b']).toBe(true)); + + // The second toggle is queued behind the first, not racing it. + expect(mockHdxServer).toHaveBeenCalledTimes(1); + expect(lastPatchBody()).toEqual({ labs: { 'lab-a': true } }); + + // Let the first finish; the queued one then sends the cumulative set. The + // fixture stands in for the persisted document once both writes have landed + // in order, so the refetch that onSettled triggers is what proves the second + // toggle survives rather than reverting. + meFixture = { labs: { 'lab-a': true, 'lab-b': true } }; + resolvePatch(); + await act(async () => { + first.resolve(); + }); + + await waitFor(() => expect(mockHdxServer).toHaveBeenCalledTimes(2)); + expect(lastPatchBody()).toEqual({ labs: { 'lab-a': true, 'lab-b': true } }); + await waitFor(() => + expect(result.current.enabled).toEqual({ + 'lab-a': true, + 'lab-b': true, + }), + ); + }); + + it('rolls back and notifies when the request fails', async () => { + meFixture = { labs: { 'lab-a': true } }; + const settle = deferPatch(); + + const { result } = renderLabs(); + await waitFor(() => expect(result.current.enabled['lab-a']).toBe(true)); + + act(() => result.current.setLabEnabled('lab-b', true)); + await waitFor(() => expect(result.current.enabled['lab-b']).toBe(true)); + + await act(async () => { + settle.reject(); + }); + + await waitFor(() => expect(result.current.enabled['lab-b']).toBe(false)); + // The pre-existing opt-in survived the rollback. + expect(result.current.enabled['lab-a']).toBe(true); + expect(mockShow).toHaveBeenCalledWith( + expect.objectContaining({ color: 'red' }), + ); + }); +}); + +describe('useIsLabEnabled', () => { + function renderIsEnabled(labId: string) { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + return renderHook(() => useIsLabEnabled(labId), { + wrapper: ({ children }) => ( + + {children} + + ), + }); + } + + it('returns true only for an enabled lab', async () => { + meFixture = { labs: { 'lab-a': true } }; + + const { result } = renderIsEnabled('lab-a'); + + await waitFor(() => expect(result.current).toBe(true)); + }); + + it('returns false for an id with no registry entry', async () => { + meFixture = { labs: { 'retired-lab': true } }; + + const { result } = renderIsEnabled('retired-lab'); + + // Deleting a registry entry is safe: the gate goes false rather than + // throwing. The flip side is that a typo fails silently. + await waitFor(() => expect(result.current).toBe(false)); + }); +}); diff --git a/packages/app/src/labs/registry.ts b/packages/app/src/labs/registry.ts new file mode 100644 index 0000000000..92fa00e77e --- /dev/null +++ b/packages/app/src/labs/registry.ts @@ -0,0 +1,47 @@ +/** + * The HyperDX Labs registry — the single source of truth for which experiments + * exist and how they're described to users. + * + * Adding a lab is intended to be a one-file change: add an entry here, then + * gate your feature with `useIsLabEnabled('your-id')`. No API change, no schema + * change, no migration. Read agent_docs/labs.md before adding one — in + * particular the graduate-or-retire rule, which is what keeps this file from + * accumulating zombie flags. + */ + +export type Lab = { + /** + * Stable kebab-case id, validated by `LabIdSchema` in common-utils. This is + * persisted on the user document, so changing it silently resets everyone's + * opt-in — treat it as permanent once shipped. + */ + id: string; + /** Short, user-facing name. Sentence case. */ + title: string; + /** + * What it does *and* what's still rough. Someone opting into unfinished work + * deserves to know where the edges are, so prefer "Span links aren't drawn + * yet and wide traces are slow" over "improved trace view". + */ + description: string; + /** Optional maturity hint shown next to the title, e.g. 'Alpha', 'Beta'. */ + badge?: string; + /** ISO day (YYYY-MM-DD). Drives the graduate-or-retire sweep. */ + addedAt: string; + /** Who decides this lab's fate. GitHub or Slack handle. */ + owner: string; +}; + +/** + * Deliberately empty. This ships the mechanism, not any experiments. + * + * Typed as `readonly Lab[]` rather than `[...] as const satisfies readonly + * Lab[]`. The const form is tempting because it would narrow lab ids to a + * literal union, making a deleted entry a compile error at every gate. But on + * an *empty* registry that union is `never`: every `lab.id` in a `.map` fails + * to typecheck, and `useIsLabEnabled` becomes uncallable, including from its + * own tests. Retirement is covered instead by the documented step of grepping + * for the id after deleting the entry — see agent_docs/labs.md. Worth + * revisiting once a few labs exist and the empty case is behind us. + */ +export const LABS: readonly Lab[] = []; diff --git a/packages/app/src/labs/useLabs.ts b/packages/app/src/labs/useLabs.ts new file mode 100644 index 0000000000..69ac810106 --- /dev/null +++ b/packages/app/src/labs/useLabs.ts @@ -0,0 +1,170 @@ +import * as React from 'react'; +import { useAtom } from 'jotai'; +import { atomWithStorage } from 'jotai/utils'; +import type { MeApiResponse, UserLabs } from '@hyperdx/common-utils/dist/types'; +import { notifications } from '@mantine/notifications'; +import { useMutation, useQueryClient } from '@tanstack/react-query'; + +import api, { hdxServer, ME_QUERY_KEY } from '@/api'; +import { IS_LOCAL_MODE } from '@/config'; +import { LABS } from '@/labs/registry'; + +export type LabsState = { + /** + * Enabled state for every lab in the current registry, keyed by lab id. Ids + * stored on the user but absent from the registry (a graduated or retired + * lab) are not surfaced here — see agent_docs/labs.md. + */ + enabled: Record; + /** + * True until the stored answer is known. Every lab reads OFF while this is + * true, so anything where an OFF -> ON flip is user-visible (a redirect, a + * default tab, a one-shot effect, a mount-time fetch) should branch on it + * rather than on `enabled` alone. Always false in local mode, which reads + * synchronously from localStorage. + */ + isLoading: boolean; + /** True while a toggle is in flight. */ + isSaving: boolean; + setLabEnabled: (labId: string, enabled: boolean) => void; +}; + +/** + * Local-mode store. Local mode has no API server and no user identity — + * `useMe()` resolves to null there — so there is no account to hang an opt-in + * on, and the choice lives in localStorage instead. Same jotai + localStorage + * shape as the other per-browser preferences in this app. + * + * This is the only reason `useLabs` branches on IS_LOCAL_MODE, and it is + * deliberately confined to this file. + */ +const localLabsAtom = atomWithStorage('hdx-labs', {}); + +/** + * Persists the caller's full lab set. Optimistic, because this backs a Switch: + * with a plain mutate-then-refetch the control would sit dead for two round + * trips. Shape mirrors src/favorites.ts, including the isMutating guard. + */ +function useUpdateUserLabs() { + const queryClient = useQueryClient(); + + return useMutation({ + // Shared with the /me query key so concurrent toggles coordinate their + // refetch rather than racing each other. + mutationKey: ME_QUERY_KEY, + // Serializes toggles: react-query runs same-scope mutations one at a time, + // in call order. Without this, two quick toggles race on the network and an + // older full-replacement payload can land last, dropping the newer lab from + // Mongo — the UI would then revert it on the next /me refetch. Each queued + // payload is built after the previous onMutate has written the cache, so + // serialized sends are cumulative rather than conflicting. + scope: { id: 'user-labs' }, + mutationFn: (labs: UserLabs) => + hdxServer('me/labs', { + method: 'PATCH', + json: { labs }, + }).json<{ labs: UserLabs }>(), + onMutate: async (labs: UserLabs) => { + // Cancel outgoing /me refetches so they can't overwrite our optimistic update + await queryClient.cancelQueries({ queryKey: ME_QUERY_KEY }); + + const previous = queryClient.getQueryData( + ME_QUERY_KEY, + ); + + queryClient.setQueryData(ME_QUERY_KEY, old => + old ? { ...old, labs } : old, + ); + + return { previous }; + }, + onError: (_err, _labs, context) => { + if (context !== undefined) { + queryClient.setQueryData(ME_QUERY_KEY, context.previous); + } + notifications.show({ + color: 'red', + message: 'Failed to update HyperDX Labs', + }); + }, + onSettled: () => { + // Only refetch once the last in-flight toggle settles, so a /me refetch + // carrying partially-committed state can't clobber a still-pending + // optimistic update from another toggle. + if (queryClient.isMutating({ mutationKey: ME_QUERY_KEY }) === 1) { + queryClient.invalidateQueries({ queryKey: ME_QUERY_KEY }); + } + }, + }); +} + +/** + * The single seam for HyperDX Labs state. Local-mode branching, loading + * semantics, and the write all live here — consumers gating a feature should + * use {@link useIsLabEnabled} instead. + */ +export function useLabs(): LabsState { + const { data: me, isPending } = api.useMe(); + const { mutate: updateLabs, isPending: isSaving } = useUpdateUserLabs(); + const [localLabs, setLocalLabs] = useAtom(localLabsAtom); + + // The one branch: an account when there is one, this browser when there isn't. + const stored = IS_LOCAL_MODE ? localLabs : me?.labs; + + const enabled = React.useMemo(() => { + // Derived from the registry, not from what's stored: an id the registry no + // longer knows about can never reach a consumer or the modal. + const result: Record = {}; + for (const lab of LABS) { + // `=== true` rather than truthiness. A key like `constructor` passes the + // lab-id regex and is inherited from Object.prototype, where it is + // truthy; only an own `true` should read as enabled. + result[lab.id] = stored?.[lab.id] === true; + } + return result; + }, [stored]); + + const setLabEnabled = React.useCallback( + (labId: string, value: boolean) => { + // Full-replace payload rebuilt from the registry. Two things fall out of + // this for free: ids of retired labs are pruned, and only enabled entries + // are stored, which keeps the document small. + const next: UserLabs = {}; + for (const lab of LABS) { + const isOn = lab.id === labId ? value : stored?.[lab.id] === true; + if (isOn) { + next[lab.id] = true; + } + } + + if (IS_LOCAL_MODE) { + setLocalLabs(next); + return; + } + updateLabs(next); + }, + [stored, updateLabs, setLocalLabs], + ); + + return { + enabled, + // localStorage is read synchronously, so local mode is never pending. + isLoading: !IS_LOCAL_MODE && isPending, + isSaving: IS_LOCAL_MODE ? false : isSaving, + setLabEnabled, + }; +} + +/** + * Whether the current user has opted into a lab. Returns `false` while the + * stored set is still loading; if that transition is user-visible, use + * {@link useLabs} and branch on `isLoading`. + * + * An id with no registry entry always reads `false`, which is what makes + * deleting an entry safe — but it also means a typo fails silently, so grep for + * the id after you delete one. See agent_docs/labs.md. + */ +export function useIsLabEnabled(labId: string): boolean { + const { enabled } = useLabs(); + return enabled[labId] === true; +} diff --git a/packages/app/tests/e2e/components/LabsModalComponent.ts b/packages/app/tests/e2e/components/LabsModalComponent.ts new file mode 100644 index 0000000000..f11045fa69 --- /dev/null +++ b/packages/app/tests/e2e/components/LabsModalComponent.ts @@ -0,0 +1,90 @@ +/** + * LabsModalComponent - the "HyperDX Labs" modal opened from the nav user menu. + * + * Toggling a lab is deliberately not covered here: with an empty registry there + * is nothing to toggle. When the first lab lands, add a `setLab(id, enabled)` + * helper alongside `labSwitch` and assert the choice survives a reload — that is + * the test worth having. See agent_docs/labs.md. + */ +import { expect, Locator, Page } from '@playwright/test'; + +export class LabsModalComponent { + readonly page: Page; + private readonly userMenuTrigger: Locator; + private readonly menuItem: Locator; + private readonly modal: Locator; + private readonly content: Locator; + private readonly emptyState: Locator; + + constructor(page: Page) { + this.page = page; + this.userMenuTrigger = page.locator('[data-testid="user-menu-trigger"]'); + this.menuItem = page.locator('[data-testid="hyperdx-labs-menu-item"]'); + this.modal = page.locator('[data-testid="labs-modal"]'); + this.content = page.locator('[data-testid="labs-modal-content"]'); + this.emptyState = page.locator('[data-testid="labs-empty-state"]'); + } + + /** + * The modal root. Always present in the DOM with zero dimensions, so assert + * `toBeHidden()` on this for closed state and use {@link content} for open. + */ + get container() { + return this.modal; + } + + /** The modal body — this is what actually reads as visible when open. */ + get body() { + return this.content; + } + + /** The user-menu entry that opens this modal. */ + get trigger() { + return this.menuItem; + } + + /** Opens the nav user menu and waits for the Labs entry to be clickable. */ + async openUserMenu() { + await this.userMenuTrigger.scrollIntoViewIfNeeded(); + await this.userMenuTrigger.waitFor({ state: 'attached' }); + await this.userMenuTrigger.click({ timeout: 10000 }); + await expect(this.menuItem).toBeVisible(); + } + + /** Opens the modal from the nav user menu. */ + async open() { + await this.openUserMenu(); + await this.menuItem.click(); + await expect(this.content).toBeVisible(); + } + + /** + * A lab's checkbox input — use for state assertions (`toBeChecked()`). + * Mantine hides it behind an aria-hidden track, so it is not clickable; + * use {@link setLab} to change it. + */ + labSwitch(labId: string) { + return this.page.locator(`[data-testid="lab-switch-${labId}"]`); + } + + /** Toggles a lab and waits for the write to be persisted. */ + async setLab(labId: string, enabled: boolean) { + const input = this.labSwitch(labId); + if ((await input.isChecked()) === enabled) { + return; + } + + const persisted = this.page.waitForResponse( + r => r.url().includes('/me/labs') && r.request().method() === 'PATCH', + ); + // The label, not the input: the input is visually hidden. + await this.page.locator(`[data-testid="lab-toggle-${labId}"]`).click(); + const response = await persisted; + expect(response.status()).toBe(200); + await expect(input).toBeChecked({ checked: enabled }); + } + + async expectEmptyState() { + await expect(this.emptyState).toBeVisible(); + } +} diff --git a/packages/app/tests/e2e/core/navigation.spec.ts b/packages/app/tests/e2e/core/navigation.spec.ts index 08738a7da4..358add4dce 100644 --- a/packages/app/tests/e2e/core/navigation.spec.ts +++ b/packages/app/tests/e2e/core/navigation.spec.ts @@ -1,3 +1,4 @@ +import { LabsModalComponent } from '../components/LabsModalComponent'; import { expect, test } from '../utils/base-test'; test.describe('Navigation', { tag: ['@core'] }, () => { @@ -109,6 +110,26 @@ test.describe('Navigation', { tag: ['@core'] }, () => { //todo: Add tests that verify user pref behavior }); + test('should open HyperDX Labs from the user menu', async ({ page }) => { + const labsModal = new LabsModalComponent(page); + + await test.step('Wait for page to load', async () => { + await expect( + page.locator('[data-testid="nav-link-search"]'), + ).toBeVisible(); + }); + + await test.step('Open the Labs modal from the user menu', async () => { + await labsModal.open(); + }); + + await test.step('Verify the empty registry renders its empty state', async () => { + // No labs ship with the mechanism, so this is the expected content. + // Replace with a toggle assertion when the first lab lands. + await labsModal.expectEmptyState(); + }); + }); + test('should open help menu', async ({ page }) => { await test.step('Navigate to and click help menu trigger', async () => { // Wait for page to be fully loaded first diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index 96a9883bff..21dee2ed36 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -2403,6 +2403,77 @@ export type InstallationApiResponse = z.infer< typeof InstallationApiResponseSchema >; +// Labs (per-user opt-in experiments) +// +// The lab *registry* — the ids and their UI copy — deliberately lives in +// packages/app/src/labs/registry.ts, not here. Adding a lab has to be a +// one-file change or the mechanism won't get used, and the server gains +// nothing from knowing the id list: the value space is `boolean` and the blast +// radius is the requester's own document. So what follows is a *shape* +// contract, not an allow-list. It bounds what a user can store without +// enumerating ids. +// +// The trade-off, stated plainly: the server cannot tell a typo'd lab id from a +// real one. That's caught client-side instead, by the registry unit test that +// parses every id against LabIdSchema. See agent_docs/labs.md. +export const LAB_ID_MAX_LENGTH = 64; +export const LABS_MAX_KEYS = 64; + +/** + * Strict kebab-case. This is a storage-safety rule, not cosmetics: it excludes + * `$` and `.` (Mongo operator and dotted-path characters) and `_` (so + * `__proto__` and friends are unrepresentable as keys). + * + * Split into a plain character class plus a dash-placement check rather than the + * idiomatic `^[a-z0-9]+(?:-[a-z0-9]+)*$`. That one-liner is in fact linear — + * the `-` separator is disjoint from `[a-z0-9]`, so there is no ambiguity to + * backtrack over — but it has the `(x+)*` shape that ReDoS linters flag, and on + * a validator guarding Mongo keys it is better to not have the argument than to + * suppress it. + */ +export const LabIdSchema = z + .string() + .min(1) + .max(LAB_ID_MAX_LENGTH) + .regex( + /^[a-z0-9-]+$/, + 'Lab ids may only contain lowercase letters, numbers and dashes', + ) + .refine( + id => !id.startsWith('-') && !id.endsWith('-') && !id.includes('--'), + { message: 'Lab ids must be kebab-case' }, + ); + +/** + * An enabled-set: a key present with `true` is on, an absent key is off. + * `false` is accepted on the write path so an older client is never rejected, + * but the current client only ever persists `true` entries. + */ +export const UserLabsSchema = z + .record(LabIdSchema, z.boolean()) + .refine(labs => Object.keys(labs).length <= LABS_MAX_KEYS, { + message: `A user may store at most ${LABS_MAX_KEYS} lab entries`, + }); + +export type UserLabs = z.infer; + +/** + * Full replace, not a patch: the client always holds the whole registry, so it + * can always compute the complete desired set — and that is what makes ids + * from retired labs self-pruning. See agent_docs/labs.md. + */ +export const UpdateUserLabsRequestSchema = z.object({ labs: UserLabsSchema }); + +export type UpdateUserLabsRequest = z.infer; + +export const UpdateUserLabsApiResponseSchema = z.object({ + labs: UserLabsSchema, +}); + +export type UpdateUserLabsApiResponse = z.infer< + typeof UpdateUserLabsApiResponseSchema +>; + // Me export const MeApiResponseSchema = z.object({ accessKey: z.string(), @@ -2418,6 +2489,13 @@ export const MeApiResponseSchema = z.object({ }).merge(TeamClickHouseSettingsSchema), usageStatsEnabled: z.boolean(), aiAssistantEnabled: z.boolean(), + /** + * Enabled-set of lab opt-ins. Optional because a rolling deploy can serve + * this from an older API pod that doesn't know about labs yet — the client + * already reads it as `me?.labs?.[id] === true`, so this is honest typing + * rather than a lie the hook would have to defend against. + */ + labs: UserLabsSchema.optional(), }); export type MeApiResponse = z.infer;