From 71ee156dacd16e236590ccb5b1d936d0c414a771 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Tue, 28 Jul 2026 10:11:59 -0700 Subject: [PATCH] feat(studio): browser canary binding + leaf subpath imports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the Studio (browser) binding so a canary can span the CLI and the editor, and fixes a bundling mistake the studio test suite caught. ## The binding Same public API as the CLI — `isCanaryEnabled("name")` — so a call site reads identically whether it runs in Node or the browser. Three inputs differ: - UNIT ID: `resolveStudioDistinctId()`, which already adopts `window.__HF_CLI_DISTINCT_ID` when the CLI launched Studio. A CLI-launched Studio therefore lands in the SAME cohort as the CLI: a rollout spanning render and editor is coherent for that user instead of enrolling their terminal but not their editor. A test pins that the id is passed through unmodified — prefixing or re-hashing it would silently break that parity. - OVERRIDE: no `process.env` in a page, so `?hf_canary_=on` mirrored into sessionStorage. Session scope is deliberate. A URL is the right carrier (shareable — "support: open this link"), but persisting a URL-borne override to localStorage would let one click silently pin a browser into a cohort forever, long after anyone remembers why. Closing the tab is the reset; `=reset` clears it explicitly. - EXCLUSION: `navigator.webdriver` stands in for the CLI's `is_ci`. Automated browsers mint a fresh localStorage id per run, so they would hop cohorts between runs — noise in the signal, nothing learned about real users. An override still reaches them, which is how you test a canary under Playwright. Studio's `trackEvent` now attaches `canaries` to every event, mirroring the CLI. ## The bundling fix Importing the `@hyperframes/core` barrel into studio browser code broke two unrelated hook test files with an esbuild TextEncoder invariant violation. The barrel re-exports the whole core surface (parsers, lint, studio-server), so it drags a Node-oriented dependency graph into a browser bundle — the test failure was the symptom, the bundle bloat was the bug. `@hyperframes/core` now exposes `./canary` and `./canary-registry`, declared in packages/core/package-subpaths.json (the generated source of truth for exports — hand-editing package.json is reverted by the sync script) and marked `environments: [browser, bun, node]`. Both the studio AND cli bindings import the leaf modules; the CLI gets the same benefit for a different reason, since this resolves on the startup path — the reason the producer is lazily loaded. Verified: the two hook files pass again; 269 studio files / 2982 tests, 98 core / 1433, 166 cli / 2194 green, `bun run lint` clean including the subpath check. Fault-injection confirms both design decisions are pinned — swapping session for local storage fails the scope test, prefixing the unit id fails the CLI/Studio cohort-parity test. Co-Authored-By: Claude Opus 5 (1M context) --- packages/cli/src/telemetry/canary.test.ts | 6 +- packages/cli/src/telemetry/canary.ts | 13 +- packages/core/package-subpaths.json | 12 ++ packages/core/package.json | 20 +++ packages/studio/src/telemetry/canary.test.ts | 174 +++++++++++++++++++ packages/studio/src/telemetry/canary.ts | 152 ++++++++++++++++ packages/studio/src/telemetry/client.ts | 6 +- 7 files changed, 372 insertions(+), 11 deletions(-) create mode 100644 packages/studio/src/telemetry/canary.test.ts create mode 100644 packages/studio/src/telemetry/canary.ts diff --git a/packages/cli/src/telemetry/canary.test.ts b/packages/cli/src/telemetry/canary.test.ts index 0fca15134..8912a47d5 100644 --- a/packages/cli/src/telemetry/canary.test.ts +++ b/packages/cli/src/telemetry/canary.test.ts @@ -12,8 +12,10 @@ vi.mock("./system.js", () => ({ // The registry is data; pin a known shape so these tests don't move when a // real canary is added or ramped. -vi.mock("@hyperframes/core", async () => { - const actual = await vi.importActual("@hyperframes/core"); +vi.mock("@hyperframes/core/canary-registry", async () => { + const actual = await vi.importActual( + "@hyperframes/core/canary-registry", + ); return { ...actual, CANARIES: [ diff --git a/packages/cli/src/telemetry/canary.ts b/packages/cli/src/telemetry/canary.ts index 69d19b4db..53eca75ae 100644 --- a/packages/cli/src/telemetry/canary.ts +++ b/packages/cli/src/telemetry/canary.ts @@ -17,14 +17,11 @@ * the feature's own code. */ -import { - CANARIES, - canaryEnvVar, - evaluateCanary, - findCanary, - parseCanaryOverride, - type CanaryDecision, -} from "@hyperframes/core"; +// Leaf subpath imports, not the "@hyperframes/core" barrel: this resolves on +// the CLI startup path, and the barrel pulls the whole core surface. Same +// reason the producer is lazily loaded. +import { evaluateCanary, parseCanaryOverride, type CanaryDecision } from "@hyperframes/core/canary"; +import { CANARIES, canaryEnvVar, findCanary } from "@hyperframes/core/canary-registry"; import { readConfig } from "./config.js"; import { getSystemMeta } from "./system.js"; diff --git a/packages/core/package-subpaths.json b/packages/core/package-subpaths.json index 62bc92fe0..2a61955dc 100644 --- a/packages/core/package-subpaths.json +++ b/packages/core/package-subpaths.json @@ -14,6 +14,18 @@ "types": null, "environments": ["browser", "bun", "node"] }, + "./canary": { + "source": "./src/canary.ts", + "runtime": "./dist/canary.js", + "types": "./dist/canary.d.ts", + "environments": ["browser", "bun", "node"] + }, + "./canary-registry": { + "source": "./src/canaryRegistry.ts", + "runtime": "./dist/canaryRegistry.js", + "types": "./dist/canaryRegistry.d.ts", + "environments": ["browser", "bun", "node"] + }, "./beats": { "source": "./src/beats/index.ts", "runtime": "./dist/beats/index.js", diff --git a/packages/core/package.json b/packages/core/package.json index a4454202a..f8d536140 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -28,6 +28,18 @@ "types": "./src/index.ts" }, "./package.json": "./package.json", + "./canary": { + "bun": "./src/canary.ts", + "node": "./dist/canary.js", + "import": "./src/canary.ts", + "types": "./src/canary.ts" + }, + "./canary-registry": { + "bun": "./src/canaryRegistry.ts", + "node": "./dist/canaryRegistry.js", + "import": "./src/canaryRegistry.ts", + "types": "./src/canaryRegistry.ts" + }, "./beats": { "bun": "./src/beats/index.ts", "node": "./dist/beats/index.js", @@ -298,6 +310,14 @@ "types": "./dist/index.d.ts" }, "./package.json": "./package.json", + "./canary": { + "import": "./dist/canary.js", + "types": "./dist/canary.d.ts" + }, + "./canary-registry": { + "import": "./dist/canaryRegistry.js", + "types": "./dist/canaryRegistry.d.ts" + }, "./beats": { "import": "./dist/beats/index.js", "types": "./dist/beats/index.d.ts" diff --git a/packages/studio/src/telemetry/canary.test.ts b/packages/studio/src/telemetry/canary.test.ts new file mode 100644 index 000000000..ee91abba0 --- /dev/null +++ b/packages/studio/src/telemetry/canary.test.ts @@ -0,0 +1,174 @@ +// @vitest-environment happy-dom + +import { describe, expect, it, vi, beforeEach, afterEach } from "vitest"; +import { evaluateCanary } from "@hyperframes/core/canary"; + +// Pin the registry: real entries move as rollouts ramp, and these tests are +// about the BINDING (does the browser supply the right three inputs?), not +// about whichever canaries happen to be live today. +vi.mock("@hyperframes/core/canary-registry", async () => { + const actual = await vi.importActual( + "@hyperframes/core/canary-registry", + ); + const defs = [ + { + name: "on-everywhere", + percentage: 100, + description: "", + owner: "t", + sunsetAfter: "2099-01-01", + }, + { + name: "off-everywhere", + percentage: 0, + description: "", + owner: "t", + sunsetAfter: "2099-01-01", + }, + ]; + return { ...actual, CANARIES: defs, findCanary: (n: string) => defs.find((d) => d.name === n) }; +}); + +const { + isCanaryEnabled, + resolveCanary, + activeCanaryNames, + canaryParamName, + __resetStudioCanaryCacheForTests, +} = await import("./canary"); +const { resolveStudioDistinctId, __resetStudioDistinctIdForTests } = await import("./distinctId"); + +function setSearch(search: string): void { + window.history.replaceState({}, "", `/${search}`); +} + +beforeEach(() => { + localStorage.clear(); + sessionStorage.clear(); + setSearch(""); + delete window.__HF_CLI_DISTINCT_ID; + Object.defineProperty(navigator, "webdriver", { value: false, configurable: true }); + __resetStudioCanaryCacheForTests(); + __resetStudioDistinctIdForTests(); +}); + +afterEach(() => { + setSearch(""); + __resetStudioCanaryCacheForTests(); + __resetStudioDistinctIdForTests(); +}); + +describe("studio canary binding", () => { + it("reads the percentage from the shared registry", () => { + expect(isCanaryEnabled("on-everywhere")).toBe(true); + expect(isCanaryEnabled("off-everywhere")).toBe(false); + }); + + it("an unregistered name is off, not a throw — a typo must not break the editor", () => { + expect(isCanaryEnabled("nope")).toBe(false); + expect(resolveCanary("nope").reason).toBe("out_of_cohort"); + }); + + it("derives the query param from the canary name", () => { + expect(canaryParamName("de-parallel-router")).toBe("hf_canary_de_parallel_router"); + }); +}); + +describe("URL override", () => { + it("turns a canary on and off from the query string", () => { + setSearch("?hf_canary_off_everywhere=on"); + expect(resolveCanary("off-everywhere")).toMatchObject({ enabled: true, reason: "forced_on" }); + + __resetStudioCanaryCacheForTests(); + setSearch("?hf_canary_on_everywhere=off"); + expect(resolveCanary("on-everywhere")).toMatchObject({ enabled: false, reason: "forced_off" }); + }); + + it("survives losing the query string, so in-app navigation keeps the override", () => { + setSearch("?hf_canary_off_everywhere=on"); + expect(isCanaryEnabled("off-everywhere")).toBe(true); + + // Navigate away from the param — a real SPA drops it constantly. + __resetStudioCanaryCacheForTests(); + setSearch(""); + expect(isCanaryEnabled("off-everywhere")).toBe(true); + }); + + it("is session-scoped, not persisted to localStorage", () => { + // A URL-borne override must not silently pin a browser into a cohort + // forever; closing the tab is the reset. + setSearch("?hf_canary_off_everywhere=on"); + expect(isCanaryEnabled("off-everywhere")).toBe(true); + expect(JSON.stringify(localStorage).includes("canary")).toBe(false); + expect(sessionStorage.length).toBeGreaterThan(0); + }); + + it("=reset clears a stored override", () => { + setSearch("?hf_canary_off_everywhere=on"); + expect(isCanaryEnabled("off-everywhere")).toBe(true); + + __resetStudioCanaryCacheForTests(); + setSearch("?hf_canary_off_everywhere=reset"); + expect(isCanaryEnabled("off-everywhere")).toBe(false); + + __resetStudioCanaryCacheForTests(); + setSearch(""); + expect(isCanaryEnabled("off-everywhere")).toBe(false); + }); +}); + +describe("automated browsers", () => { + it("are excluded from percentage enrolment", () => { + Object.defineProperty(navigator, "webdriver", { value: true, configurable: true }); + expect(resolveCanary("on-everywhere")).toMatchObject({ enabled: false, reason: "excluded" }); + }); + + it("still honour an explicit override, so a canary can be tested under automation", () => { + Object.defineProperty(navigator, "webdriver", { value: true, configurable: true }); + setSearch("?hf_canary_on_everywhere=on"); + expect(resolveCanary("on-everywhere")).toMatchObject({ enabled: true, reason: "forced_on" }); + }); +}); + +describe("cohort identity", () => { + it("buckets on the Studio distinct id unmodified — so a CLI-launched Studio shares the CLI's cohort", () => { + // distinctId.ts adopts window.__HF_CLI_DISTINCT_ID when the CLI launched + // Studio. This asserts the binding passes that id through untouched: if it + // prefixed or re-hashed it, the editor would land in a different cohort + // than the terminal for the same user, and a rollout spanning both would + // be incoherent. + const cliId = "db0c1f4a-b95e-4c35-90c6-1a15bd76f717"; + window.__HF_CLI_DISTINCT_ID = cliId; + __resetStudioDistinctIdForTests(); + __resetStudioCanaryCacheForTests(); + + expect(resolveStudioDistinctId()).toBe(cliId); + // 50% so the answer is id-dependent rather than trivially true. + const viaBinding = resolveCanary("on-everywhere").bucket; + const direct = evaluateCanary({ + feature: "on-everywhere", + unitId: cliId, + percentage: 100, + }).bucket; + expect(viaBinding).toBe(direct); + }); + + it("memoizes so a decision cannot change mid-session", () => { + expect(isCanaryEnabled("off-everywhere")).toBe(false); + // A late override must NOT flip a component that already rendered. + setSearch("?hf_canary_off_everywhere=on"); + expect(isCanaryEnabled("off-everywhere")).toBe(false); + __resetStudioCanaryCacheForTests(); + expect(isCanaryEnabled("off-everywhere")).toBe(true); + }); +}); + +describe("telemetry", () => { + it("reports enrolled canaries, undefined when none", () => { + expect(activeCanaryNames()).toBe("on-everywhere"); + + __resetStudioCanaryCacheForTests(); + setSearch("?hf_canary_on_everywhere=off"); + expect(activeCanaryNames()).toBeUndefined(); + }); +}); diff --git a/packages/studio/src/telemetry/canary.ts b/packages/studio/src/telemetry/canary.ts new file mode 100644 index 000000000..f1494390e --- /dev/null +++ b/packages/studio/src/telemetry/canary.ts @@ -0,0 +1,152 @@ +// --------------------------------------------------------------------------- +// Studio (browser) binding for the shared canary registry. +// +// `@hyperframes/core` owns the decision and is deliberately pure — the caller +// supplies the unit id, the override and the exclusion. This file supplies +// those three from the browser, mirroring `packages/cli/src/telemetry/canary.ts` +// for the CLI. The public API is deliberately identical on both surfaces: +// +// import { isCanaryEnabled } from "../telemetry/canary"; +// if (isCanaryEnabled("my-feature")) { ... } +// +// so a call site reads the same whether it runs in Node or the browser, and a +// canary can span both. +// +// Three things differ from the CLI, each for a reason: +// +// 1. UNIT ID — `resolveStudioDistinctId()` instead of the CLI's config file. +// That function already adopts `window.__HF_CLI_DISTINCT_ID` when the CLI +// launched Studio, so a CLI-launched Studio lands in the SAME cohort as the +// CLI itself: a rollout spanning render and editor is coherent for that +// user instead of enrolling their terminal but not their editor. +// +// 2. OVERRIDE — there is no `process.env` in a page, so the override is a URL +// query param mirrored into sessionStorage (see `readOverride`). +// +// 3. EXCLUSION — `navigator.webdriver` stands in for the CLI's `is_ci`. +// Automated browsers mint a fresh localStorage id per run, so their ids are +// ephemeral and they would hop cohorts between runs — noise in the rollout +// signal, and nothing learned about real users. +// --------------------------------------------------------------------------- + +// Deep subpath imports, NOT the "@hyperframes/core" barrel. Studio is a +// browser bundle, and the barrel re-exports the whole core surface (parsers, +// lint, studio-server); pulling that in here drags a Node-oriented dependency +// graph into the bundle. These two modules are pure and leaf. +import { evaluateCanary, parseCanaryOverride, type CanaryDecision } from "@hyperframes/core/canary"; +import { CANARIES, findCanary } from "@hyperframes/core/canary-registry"; +import { resolveStudioDistinctId } from "./distinctId"; +import { safeSessionStorage } from "../utils/safeStorage"; + +/** `my-feature` → `hf_canary_my_feature`, the query param and storage key. */ +export function canaryParamName(name: string): string { + return `hf_canary_${name.toLowerCase().replace(/[^a-z0-9]+/g, "_")}`; +} + +const STORAGE_PREFIX = "hyperframes-studio:canary:"; + +/** + * Resolve a manual override for one canary. + * + * `?hf_canary_my_feature=on` (also off/true/false/1/0/yes/no), mirrored into + * sessionStorage so it survives in-app navigation and reloads within the tab. + * + * SESSION scope, not local, is the deliberate part. A URL is the right carrier + * — it is shareable, which is what "support: open this link" and "QA: repro + * with this on" actually need. But persisting a URL-borne override to + * localStorage would mean one click silently pins that browser into a cohort + * forever, long after anyone remembers why. Session scope keeps the link + * useful and lets closing the tab be the reset. + * + * `?hf_canary_my_feature=reset` clears it explicitly. + */ +function readOverride(name: string): boolean | undefined { + if (typeof window === "undefined") return undefined; + const key = canaryParamName(name); + const storageKey = `${STORAGE_PREFIX}${name}`; + const store = safeSessionStorage(); + + let raw: string | null = null; + try { + raw = new URLSearchParams(window.location.search).get(key); + } catch { + raw = null; + } + + if (raw !== null) { + if (raw.trim().toLowerCase() === "reset") { + store?.removeItem(storageKey); + return undefined; + } + // Persist for the tab session so the override outlives the query string. + try { + store?.setItem(storageKey, raw); + } catch { + /* storage full / blocked — the param still applies to this page load */ + } + return parseCanaryOverride(raw); + } + + return parseCanaryOverride(store?.getItem(storageKey) ?? undefined); +} + +/** + * Automated browser? The browser analog of the CLI's CI exclusion. + * `navigator.webdriver` is set by Playwright, Puppeteer and Selenium. + */ +function isAutomatedBrowser(): boolean { + if (typeof navigator === "undefined") return false; + return navigator.webdriver === true; +} + +/** + * Memoized per page load, for the same reason the CLI memoizes per process: a + * canary must not change its mind mid-session. A component that mounted + * enrolled has to stay enrolled, and the telemetry has to agree with what the + * user actually saw. + */ +const decisions = new Map(); + +/** Test-only: drop memoized decisions so cases don't leak into each other. */ +export function __resetStudioCanaryCacheForTests(): void { + decisions.clear(); +} + +/** + * Full decision including the reason. An unregistered name resolves to off + * rather than throwing — a typo in a rollout control must never break the + * editor. + */ +export function resolveCanary(name: string): CanaryDecision { + const cached = decisions.get(name); + if (cached) return cached; + + const definition = findCanary(name); + const decision: CanaryDecision = definition + ? evaluateCanary({ + feature: definition.name, + unitId: resolveStudioDistinctId(), + percentage: definition.percentage, + override: readOverride(definition.name), + exclude: isAutomatedBrowser(), + }) + : { enabled: false, reason: "out_of_cohort" }; + + decisions.set(name, decision); + return decision; +} + +/** Is this canary on for this Studio install? The everyday call. */ +export function isCanaryEnabled(name: string): boolean { + return resolveCanary(name).enabled; +} + +/** + * Comma-joined names of the canaries this install is enrolled in, or undefined + * when none — attached to every Studio event so any metric can be split by + * cohort, exactly as the CLI does. + */ +export function activeCanaryNames(): string | undefined { + const active = CANARIES.filter((c) => resolveCanary(c.name).enabled).map((c) => c.name); + return active.length > 0 ? active.join(",") : undefined; +} diff --git a/packages/studio/src/telemetry/client.ts b/packages/studio/src/telemetry/client.ts index cdeccfb45..b872ceb7a 100644 --- a/packages/studio/src/telemetry/client.ts +++ b/packages/studio/src/telemetry/client.ts @@ -6,6 +6,7 @@ import { getAnonymousId, hasShownNotice, isOptedOut, markNoticeShown } from "./config"; import { getBrowserSystemMeta } from "./system"; +import { activeCanaryNames } from "./canary"; // Write-only PostHog project key, safe to embed in client code. const POSTHOG_API_KEY = "phc_zjjbX0PnWxERXrMHhkEJWj9A9BhGVLRReICgsfTMmpx"; @@ -73,7 +74,10 @@ export function trackEvent(event: string, properties: EventProperties = {}): voi const sys = getBrowserSystemMeta(); eventQueue.push({ event, - properties: { ...properties, ...sys }, + // `canaries` mirrors the CLI: the cohorts this install is enrolled in, on + // EVERY event so any metric can be split by cohort. Resolved after the + // shouldTrack guard, so opted-out users never pay for it. + properties: { ...properties, ...sys, canaries: activeCanaryNames() }, timestamp: new Date().toISOString(), });