diff --git a/docs/contributing/canary-rollouts.mdx b/docs/contributing/canary-rollouts.mdx new file mode 100644 index 0000000000..c224b2f0b2 --- /dev/null +++ b/docs/contributing/canary-rollouts.mdx @@ -0,0 +1,310 @@ +--- +title: "Canary rollouts" +description: "Ship a change to a percentage of installs instead of all-or-nothing." +--- + +Most flags in this repo are binary: a feature is off (and therefore never +exercised on real traffic) or on for everyone (and therefore a fleet-wide +bet). A canary is the rung in between — the same change, enabled for a stable +slice of installs, ramped as the signal holds. + + + **Canaries are part of telemetry, not a separate system.** A canary is a + *measured* rollout: a slice is enrolled specifically so it can be compared + against everyone else, and the comparison is what makes ramping safe. An + install that reports nothing cannot be compared, so **opting out of telemetry + opts you out of canaries** — via `hyperframes telemetry disable`, + `HYPERFRAMES_NO_TELEMETRY=1`, `DO_NOT_TRACK=1`, a dev build, or Studio's + `hyperframes-studio:telemetryDisabled`. See the + [`telemetry` command](/packages/cli#telemetry) for what is collected and how + to turn it off; everything on this page sits behind that switch. + + +## Add one + +**1. Register it at 0%.** In `packages/core/src/canaryRegistry.ts`: + +```ts +{ + name: "my-feature", + percentage: 0, + description: "One line on what turning this on actually changes.", + owner: "your-handle", + sunsetAfter: "2026-12-01", +} +``` + +At `0` it is inert, so this lands safely on its own. + +**2. Read it at the decision point.** + +```ts +import { isCanaryEnabled } from "../telemetry/canary.js"; + +if (isCanaryEnabled("my-feature")) { + // new path +} else { + // existing path +} +``` + +That is the whole API. The percentage lives in the registry, never at the +call site. + +**3. Ramp it** by editing `percentage` in a patch release: `0 → 5 → 25 → 100`. + +**4. Delete it** once it is at 100 and holding — both the registry entry and +the branch it guarded. `sunsetAfter` exists to force this: a test fails once +the date passes. + +## Overriding + +```bash +HF_CANARY_MY_FEATURE=on # or off / true / false / 1 / 0 / yes / no +``` + +Upper-snake-case the name. An override always wins over the percentage, in +both directions — use it for support escalations, dogfooding, bisects, or a +panic-off. + +## Measuring + +Every telemetry event carries the assignment as a flag-shaped property: + +``` +$feature/canary-my-feature: "true" | "false" +``` + +`$feature/` is the de-facto flag-property convention in analytics +tooling, so whoever operates the telemetry backend can split any metric by +cohort with **nothing configured server-side** — while the decision itself +still happens locally and offline, which the render path requires. +(Assignments ride the same anonymous, opt-out telemetry pipeline as every +other event — and disabling telemetry disables the **enrolment**, not just the +reporting: an opted-out install is never bucketed at all. See the note at the +top of this page.) + +Two details worth knowing: + +- **Both arms are emitted.** A non-enrolled install reports `"false"`, not a + missing property. Absent means *this build predates the canary*, which is a + different fact from *this install is control* — collapsing them makes a ramp + unreadable. +- **Keys are namespaced with `canary-`.** The `$feature/` namespace is shared + with real product feature flags elsewhere in the analytics pipeline. The + infix guarantees a canary can never alias one and fight it for the same + property. + +## Cumulative exposure — the number that actually bounds blast radius + +The instantaneous share holds at the target forever: enrolment is a pure +function of `(feature, installId, percentage)` and fresh ids are uniformly +random, so ~10% of active installs and ~10% of renders are enrolled on any +given day. That part does not drift. + +**The cumulative set does grow.** Install ids churn — measured on the desktop +render population, there are ~25× more distinct ids over 30 days than in any +single day. Ten percent of a pool that keeps turning over is a steadily larger +group of installs that have been enrolled *at some point*. If a single person +cycles through N ids during a rollout, their chance of having been exposed is +`1 − (1 − p)^N`, so at 10%: 19% after two ids, 41% after five. + +So the number that bounds blast radius is *distinct installs ever enrolled +during the window*, not the instantaneous rate. Two practical consequences: + +- **Keep canaries short.** Drift compounds with time; a 5-day window at 10% is + far tighter than a 60-day one. +- **The percentage is not the safety mechanism.** It bounds *initial* exposure + and decays from there. Per-render verification and per-install circuit + breakers are what actually bound harm. A breaker's tripped state lives in + `install-state.json`, separate from `config.json` and merged back in on every + read, so a `config.json` re-mint cannot re-enrol an install into a path that + already failed it. Deleting `~/.hyperframes` clears both, deliberately — + that is the user's reset. + +For *measurement* — "is this feature better?" — churn is harmless: re-bucketing +is random, so it adds noise, not bias. It is specifically the blast-radius +guarantee that degrades. + +## Calibration: validating the mechanism in the wild + +Two inert canaries — `calibration-10` (10%) and `calibration-50` (50%) — gate +nothing and exist only to prove the mechanism behaves as designed on real +traffic. Their percentages stay FIXED for the whole window, which is what +makes check 3 below meaningful: while a percentage is constant, a cohort flip +is a bug, whereas during a real ramp a `false → true` flip is correct and +expected. + +Four checks, written down before the data arrives so the read is not post-hoc. +(The maintainers monitor these on an internal dashboard; the definitions live +here so the experiment's terms are public and fixed.) + +**1. Accuracy — does 10% mean 10%?** Measured install-weighted AND +event-weighted separately: render volume is heavily skewed toward a few heavy +installs, so the two can differ even when bucketing is perfect. Install share +is the one that must land on target. + +**2. Drift — does cumulative exposure climb?** Measured over widening windows +(1/7/14/30 days). The instantaneous share should stay flat at 10%; the +cumulative enrolled-install count should grow with id churn. + +**3. Stability — does any install ever change cohort?** MUST be zero. A single +id reporting both `true` and `false` at a fixed percentage means something is +broken: memoization, a registry edit mid-window, or two code paths +disagreeing. One innocent explanation to rule out first on a small-nonzero +read: events carry the assignment but not the decision *reason*, so a dev +toggling `HF_CANARY_CALIBRATION_10=on/off` mid-window is indistinguishable +from a real flip. Emitting a reason property is deliberately skipped — add it +only if this check comes back dirty. + +**4. Cross-surface agreement — do CLI and Studio agree for the same install?** +A CLI-launched Studio adopts the CLI's id, so the same install must report the +same cohort on both. Disagreement means the two bindings have diverged. This +compares the *value reported per surface* — any disagreement here is also a +check-3 flip, so this check's job is attribution: it isolates the flips that +are binding divergence rather than within-surface instability. + +**Independence bonus:** overlap between `calibration-10` and `calibration-50` +should be ~5% of installs (p1 x p2), not ~10% (which would mean the slices are +correlated and every canary hits the same unlucky cohort). + +### What passing looks like, and what cannot be fixed + +Checks 1, 3 and 4 should pass outright — they are properties of the design, +and failing any of them is a bug to fix before shipping a real rollout. + +Check 2 will NOT come back flat, and that is expected rather than a defect. +Per-install cohorts never flip, but a *person* who wipes their config gets a +new id and a fresh roll of the dice. Preventing that entirely needs a stable +identity across resets, and both candidates were rejected: hardware +fingerprinting correlates the cohort with hardware (fatal for a rendering +experiment, and it survives uninstall), and account identity covers only +~3.6% of local rendering installs. So the drift itself stands as a measured, +accepted limit — but its worst consequence is mitigated, and its size is now +directly measurable rather than inferred: + +- **Cohorts survive a config re-mint.** Canaries bucket on a dedicated + `bucketSeed` — not the telemetry id — held in `install-state.json` beside + `config.json` and write-once (the first install's seed wins forever). This + matters because `config.json` is rewritten on every command and every + render, and any parse/permission/IO failure makes the CLI mint a fresh + identity. The seed is never emitted, so it does not link the old id to the + new one. +- **Deleting `~/.hyperframes` clears the cohort too, deliberately.** The seed + does not live outside the config directory, so the user's reset is a real + reset. A seed that outlived it would be a persistent identifier defeating + the only lever they have — see [the removal path](#removing-your-canary-state). +- **A re-minted install cannot re-enter a path that already failed on that + machine.** The circuit breaker's tripped state is mirrored to the same + state file. +- **`install_predecessor_found`** on every event says whether this install's + mint found a previous install's state marker — i.e. `config.json` was lost + while `install-state.json` survived. Its true-share is the re-mint rate + directly; drift from a deliberate reset, a fresh machine, a container, or a + new user is not linkable by any local mechanism and is not counted. + +With the seed carryover, check 2's residual drift comes from the +unrecoverable buckets only — deliberate resets, fresh machines, containers, +and genuinely new users — and canary window lengths get picked from that +residual, not the raw turnover. + +## Behaviour worth knowing + +**Ramping is inclusive.** Widening `10 → 25` keeps everyone who was already +in the 10. Cohorts never reshuffle, so a before/after comparison stays valid +across a ramp. + +**Slices are independent per feature.** The bucket hashes `feature:seed`, +not the seed alone — two canaries at 10% select two different 10%s. If +they shared a slice, one unlucky cohort would receive every experiment at +once and no two rollouts could be read apart. + +**Cohorts are keyed to the install directory, not to `config.json`.** The +bucketing unit is a dedicated seed in `install-state.json`, inherited across +`config.json` re-mints (see the calibration section) — distinct from the +telemetry id and never emitted. It does not outlive `~/.hyperframes`. + +**A CLI-launched Studio adopts the CLI's decisions rather than re-deriving +them.** The CLI publishes `window.__HF_CLI_CANARY_DECISIONS` — a +`{ name: { enabled, forced } }` map. Re-deriving cannot agree in the cases +that matter: telemetry off (the CLI resolves `telemetry_opt_out`, but Studio's +opt-out is a separate localStorage flag it cannot see), an `HF_CANARY_*` +override (env vars never cross into the browser), or no seed injected (Studio +falls back to a different unit, so a different bucket). One render spanning +both surfaces must not run half-enrolled. + +`forced` carries the provenance, and the precedence follows from it — highest +first: + +1. A **forced** CLI decision (`HF_CANARY_*`). Wins outright, including over + this profile's opt-out, exactly as a local URL override does. +2. A local `?hf_canary_*=` override, same reasoning. +3. **This profile's telemetry opt-out.** Checked before any percentage + decision: the two surfaces have independent opt-outs, and CLI telemetry + being on says nothing about whether this browser profile agreed to be + measured. A cohort roll must never enrol an opted-out profile. +4. The CLI's percentage decision. +5. Local evaluation (standalone Studio, or a canary the CLI did not publish). + +Identity is treated differently from decisions. `__HF_CLI_DISTINCT_ID` and +`__HF_CLI_BUCKET_SEED` are injected only for a loopback `Host` — a page that +rebinds its hostname to `127.0.0.1` would otherwise read them as same-origin. +The decisions map is not identifying, so it is published regardless, which +keeps a LAN preview (`HYPERFRAMES_PREVIEW_HOST=0.0.0.0`) in agreement with the +CLI instead of silently re-deriving. + +The decisions map is published even when telemetry is off — that is the case +it exists for. It is safe to expose where the seed is not: booleans about +features, not the value cohorts are derived from. Studio still evaluates +locally when opened standalone, or for any canary the CLI did not publish. + +## Removing your canary state + +Both `config.json` and `install-state.json` live in `~/.hyperframes`, so: + +```bash +rm -rf ~/.hyperframes # clears telemetry id, canary cohorts, and breaker state +``` + +`hyperframes telemetry status` prints both paths if you want to inspect or +delete them individually. Nothing canary-related is stored anywhere else. + +**Opting out of telemetry opts you out of canaries.** A canary is a *measured* +rollout — we enrol a slice precisely so it can be compared against everyone +else. An install that sends nothing can't be compared, so enrolling it buys no +signal and only changes that user's code path, on an experimental feature, +without their knowledge. Every opt-out route counts: the persisted preference +(`hyperframes telemetry disable`), the runtime env vars +(`HYPERFRAMES_NO_TELEMETRY`, `DO_NOT_TRACK`), dev/telemetry-disabled builds, +and Studio's `hyperframes-studio:telemetryDisabled`. The decision resolves to +`telemetry_opt_out` *before* bucketing, so no cohort is assigned at all. + +An explicit `HF_CANARY_` (or `?hf_canary_=` in Studio) +override still wins — that is a deliberate local choice, and it stays the way +to exercise a canary with telemetry off. + +**It fails closed.** No install id, an unregistered name, or a CI machine all +resolve to *not enrolled*. A canary exists to bound blast radius, so "we +don't know who this is" must never mean "enrol everyone". CI is excluded +because its config is regenerated per run, so its ids are ephemeral and would +hop cohorts between runs — an explicit override still reaches it, which is +how you test a canary in CI. + +**Decisions are stable and memoized.** The same install always resolves the +same way, and the answer is fixed for the life of a process — a render that +starts enrolled finishes enrolled, and its telemetry agrees with what ran. + +**Do not rename a live canary.** The name is part of the hash, so renaming +reshuffles the cohort mid-rollout and invalidates the comparison. + +## Where it lives + +| File | Role | +| --- | --- | +| `packages/core/src/canary.ts` | Pure evaluator — no fs, no network, browser-safe | +| `packages/core/src/canaryRegistry.ts` | Every rollout, with owner and sunset date | +| `packages/cli/src/telemetry/canary.ts` | CLI binding: install id, env override, CI detection | + +The evaluator is deliberately dependency-free so studio and the embeddable +player can use it too; those surfaces need their own thin binding to supply an +id, since only the CLI has `anonymousId`. diff --git a/docs/docs.json b/docs/docs.json index 8bb5f0e6dc..505816f042 100644 --- a/docs/docs.json +++ b/docs/docs.json @@ -472,6 +472,7 @@ "contributing/release-channels", "contributing/changelog-process", "contributing/testing-local-changes", + "contributing/canary-rollouts", "contributing/studio-manual-dom-editing" ] }, diff --git a/docs/guides/feedback.mdx b/docs/guides/feedback.mdx index f7f70231ef..5d5a397119 100644 --- a/docs/guides/feedback.mdx +++ b/docs/guides/feedback.mdx @@ -199,7 +199,7 @@ Studio stores the equivalent state in `localStorage` under the `hyperframes-stud ### CLI — disable telemetry entirely -Disabling telemetry suppresses the CLI feedback prompt and all other CLI usage tracking: +Disabling telemetry suppresses the CLI feedback prompt, all other CLI usage tracking, and enrolment in any [canary rollout](/contributing/canary-rollouts) — an install that reports nothing is never picked for a staged rollout: ```bash # Via CLI command (persisted to ~/.hyperframes/config.json) diff --git a/docs/packages/cli.mdx b/docs/packages/cli.mdx index efd4b331a2..ab704e8b40 100644 --- a/docs/packages/cli.mdx +++ b/docs/packages/cli.mdx @@ -992,6 +992,13 @@ Word-level transcripts (whisper output) are grouped into readable caption cues o Telemetry collects command names, render performance, render checkpoint/error names, aggregate browser diagnostic counts, browser initialization duration and tween count, aggregate video extraction workload counts (such as extracted frame count and VFR preflight count), example choices, and system info — including a coarse environment fingerprint (OS, kernel string, CPU/memory shape, sandbox runtime such as gVisor or Docker, and the *name* of a coding agent driving the CLI when one is detected, e.g. `claude_code` / `codex` / `cursor`). The agent name is derived from the existence of well-known environment variables; their values are never read. Telemetry redacts local paths and URL query strings from render error/checkpoint messages and does **not** collect project names, video content, or environment variable values. It collects no personally identifiable information until you sign in: when you authenticate with `hyperframes auth login`, your HeyGen account email (or your username, if your account has no email) is linked to your usage so CLI activity can be associated with your account (and your prior anonymous usage is stitched to it). Nothing else personal is collected, and this only happens after you choose to sign in. Disable all telemetry with `HYPERFRAMES_NO_TELEMETRY=1` or the command above. + Telemetry state also controls **canary enrolment**: staged rollouts pick a + stable slice of installs to enable a change for, and an install that reports + nothing cannot be compared against anyone, so opting out of telemetry opts you + out of canaries too. Every route counts — `hyperframes telemetry disable`, + `HYPERFRAMES_NO_TELEMETRY=1`, `DO_NOT_TRACK=1`, and dev builds. See + [Canary rollouts](/contributing/canary-rollouts). + See [Feedback Collection](/guides/feedback) for how the periodic post-render prompt and Studio feedback bar work, what data they collect, and how to opt out. ### `skills` diff --git a/packages/cli/src/commands/render.test.ts b/packages/cli/src/commands/render.test.ts index 092860ce30..bbfc4b55c1 100644 --- a/packages/cli/src/commands/render.test.ts +++ b/packages/cli/src/commands/render.test.ts @@ -30,11 +30,14 @@ const configState = vi.hoisted( cache: Record | null; writeConfigCalls: Array>; failWrites: number; + /** Config write lands but the install-state mirror does not. */ + failMirrors: number; } => ({ disk: { telemetryEnabled: true, deParallelRouterTrialFired: true }, cache: null, writeConfigCalls: [], failWrites: 0, + failMirrors: 0, }), ); @@ -147,6 +150,22 @@ vi.mock("../telemetry/config.js", () => ({ configState.cache = { ...config }; return true; }), + // The breaker's safety path uses this rather than writeConfig, so it can + // see a mirror failure instead of having it collapsed into `true`. + writeConfigWithResult: vi.fn((config: Record) => { + configState.writeConfigCalls.push({ ...config }); + if (configState.failWrites > 0) { + configState.failWrites--; + return { ok: false, error: "mock write failure" }; + } + configState.disk = { ...config }; + configState.cache = { ...config }; + if (configState.failMirrors > 0) { + configState.failMirrors--; + return { ok: true, mirrored: false }; + } + return { ok: true }; + }), })); vi.mock("../telemetry/client.js", () => ({ @@ -216,6 +235,7 @@ describe("renderLocal browser GPU config", () => { configState.disk = { telemetryEnabled: true, deParallelRouterTrialFired: true }; configState.cache = null; configState.failWrites = 0; + configState.failMirrors = 0; configState.writeConfigCalls = []; trackingState.shouldTrack = true; trackingState.renderObservations = []; @@ -1050,6 +1070,33 @@ describe("renderLocal — DE parallel-router CLI trial", () => { expect(process.env.HF_DE_PARALLEL_ROUTER).toBeUndefined(); }); + // The config write landing is NOT enough: config.json is the copy a stale + // writer or a re-mint can erase, so a run that mirrored nothing has left the + // safety fact on the erasable store only. writeConfig() collapsed + // {ok:true, mirrored:false} to success and the loop stopped there. + it("retries when the install-state mirror fails even though config.json landed", async () => { + configState.disk = { + telemetryEnabled: true, + deParallelRouterTrialFired: false, + telemetryNoticeShown: true, + }; + configState.failMirrors = 1; // first attempt mirrors nothing + producerState.executeImpl = async (job) => { + job.perfSummary = { + resolution: { width: 100, height: 100 }, + drawElement: { parallelRouter: "reverted" }, + }; + }; + await renderLocal("/tmp/project", "/tmp/out.mp4", baseOptions); + + expect(configState.disk.deParallelRouterTrialFired).toBe(true); + // Two writes: the one whose mirror failed, then the retry that mirrored. + const firedWrites = configState.writeConfigCalls.filter( + (c) => c.deParallelRouterTrialFired === true, + ); + expect(firedWrites.length).toBeGreaterThanOrEqual(2); + }); + it("re-asserts the fired flag when the write is lost (concurrent clobber / transient failure), without re-counting the render", async () => { configState.disk = { telemetryEnabled: true, diff --git a/packages/cli/src/commands/render.ts b/packages/cli/src/commands/render.ts index d832746369..78fbd65674 100644 --- a/packages/cli/src/commands/render.ts +++ b/packages/cli/src/commands/render.ts @@ -66,6 +66,7 @@ import { readConfigFresh, recordRecentRender, writeConfig, + writeConfigWithResult, type HyperframesConfig, } from "../telemetry/config.js"; import { shouldTrack } from "../telemetry/client.js"; @@ -1253,13 +1254,23 @@ function resolveDeParallelRouterOutcome(job: RenderJob): string | undefined { */ function persistDeParallelRouterTrialFired(): boolean { const MAX_ATTEMPTS = 3; + let mirrored = false; for (let attempt = 0; attempt < MAX_ATTEMPTS; attempt++) { const config = readConfigFresh(); - if (config.deParallelRouterTrialFired) return true; + // Both stores must carry the latch, not just config.json. Checking only + // the config let a run stop early after a failed mirror — and config.json + // is the copy a stale writer or a re-mint can erase, so the durable one + // is exactly the one that was missing. The read side merges install-state + // back in, so the two together are what make the trip survive. + if (config.deParallelRouterTrialFired && mirrored) return true; config.deParallelRouterTrialFired = true; - if (!writeConfig(config)) return false; + const result = writeConfigWithResult(config); + if (!result.ok) return false; + mirrored = result.mirrored !== false; + if (mirrored) return true; + // Config landed but the mirror did not — retry rather than report success. } - return Boolean(readConfigFresh().deParallelRouterTrialFired); + return false; } /** diff --git a/packages/cli/src/commands/telemetry.test.ts b/packages/cli/src/commands/telemetry.test.ts index 3fe6150328..27e62fbc7f 100644 --- a/packages/cli/src/commands/telemetry.test.ts +++ b/packages/cli/src/commands/telemetry.test.ts @@ -37,7 +37,9 @@ async function loadTelemetryCommand(options?: { vi.doMock("../utils/env.js", () => ({ isDevMode: () => options?.devMode ?? false, })); - vi.doMock("../telemetry/transport.js", () => ({ + // The key moved to a leaf module to break a config -> policy -> transport + // -> config import cycle; policy.ts reads it from there now. + vi.doMock("../telemetry/posthogKey.js", () => ({ POSTHOG_API_KEY: options?.apiKey ?? "phc_test", })); const module = await import("./telemetry.js"); diff --git a/packages/cli/src/server/studioServer.test.ts b/packages/cli/src/server/studioServer.test.ts index 466317aff3..f11d0dee8a 100644 --- a/packages/cli/src/server/studioServer.test.ts +++ b/packages/cli/src/server/studioServer.test.ts @@ -57,3 +57,46 @@ describe("createStudioServer autoProxy plumbing", () => { expect(server.adapter.autoProxy).toBe(true); }); }); + +describe("host guarding on identity-bearing responses", () => { + const dirs: string[] = []; + let server: StudioServer | undefined; + + function tmpProject(): string { + const dir = mkdtempSync(join(tmpdir(), "hf-studio-host-test-")); + dirs.push(dir); + return dir; + } + + afterEach(() => { + server?.watcher.close(); + server = undefined; + for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }); + }); + + // NOTE: the SPA-injection branch itself is covered in telemetryIdentity.test.ts + // via buildStudioHeadScriptsForHost. It cannot be asserted here: this route + // only reaches the injection branch when packages/studio/dist is built, + // which is true locally and false in the CI test lane, so a route-level + // assertion on the returned HTML passes on a dev box and fails in CI. + + it("refuses the identity endpoint for a hostile Host", async () => { + server = createStudioServer({ projectDir: tmpProject() }); + const res = await server.app.request("/api/telemetry-identity", { + headers: { host: "evil.example.com" }, + }); + expect(res.status).toBe(403); + expect(await res.text()).not.toContain('distinctId":"'); + }); + + it("serves the identity endpoint on a loopback Host", async () => { + server = createStudioServer({ projectDir: tmpProject() }); + const res = await server.app.request("/api/telemetry-identity", { + headers: { host: "127.0.0.1:5173" }, + }); + expect(res.status).toBe(200); + // The seed is no longer served here at all — Studio gets decisions + // injected instead, so nothing needs it over HTTP. + expect(Object.keys((await res.json()) as object)).toEqual(["distinctId"]); + }); +}); diff --git a/packages/cli/src/server/studioServer.ts b/packages/cli/src/server/studioServer.ts index 358189ad63..8b5e7c10ab 100644 --- a/packages/cli/src/server/studioServer.ts +++ b/packages/cli/src/server/studioServer.ts @@ -16,7 +16,11 @@ import { loadRuntimeSourceSignature, } from "./runtimeSource.js"; import { VERSION as version } from "../version.js"; -import { buildStudioHeadScripts, resolveCliTelemetryDistinctId } from "./telemetryIdentity.js"; +import { + buildStudioHeadScriptsForHost, + identityAllowed, + resolveCliTelemetryDistinctId, +} from "./telemetryIdentity.js"; import { emitStudioRenderComplete, emitStudioRenderError } from "./studioRenderTelemetry.js"; import { isDevMode } from "../utils/env.js"; import { @@ -651,7 +655,22 @@ export function createStudioServer(options: StudioServerOptions): StudioServer { // clients that can't rely on the injected global. Returns the CLI's anonymous // distinct id (no PII) so the browser session can join the CLI's PostHog // person, or `{ distinctId: null }` when CLI telemetry is disabled. + // + // Deliberately does NOT serve `bucketSeed`. Studio gets its canary answers + // from the injected `window.__HF_CLI_CANARY_DECISIONS` (booleans, not the + // value cohorts derive from), so nothing needs the seed over HTTP — and an + // unauthenticated local endpoint is a strictly worse place for it than an + // inline script scoped to Studio's own document. + // + // Host-guarded against DNS rebinding: a remote page can point a hostname it + // controls at 127.0.0.1 and read this response as same-origin. Pinning the + // Host header to a loopback name means such a request (which carries the + // attacker's hostname) is refused. Same-origin Studio traffic always + // presents the bound loopback host. app.get("/api/telemetry-identity", (c) => { + if (!identityAllowed(c.req.header("host"))) { + return c.json({ error: "forbidden" }, 403); + } return c.json({ distinctId: resolveCliTelemetryDistinctId() }); }); @@ -795,7 +814,17 @@ export function createStudioServer(options: StudioServerOptions): StudioServer { // Inject before the studio bundle runs. Identity script first (see // buildStudioHeadScripts) so the CLI distinct id is on `window` by the time // telemetry init reads it. - const headScript = buildStudioHeadScripts(buildRuntimeEnvScript()); + // + // Host-guarded for the same reason /api/telemetry-identity is, and it has + // to be checked HERE too: guarding only the endpoint leaves this route as + // an open side door, since a rebound origin can simply fetch `/` and read + // the same distinct id and seed out of the returned HTML. + // + // Only IDENTITY is withheld from an untrusted Host. The canary decisions + // map still goes out — it is non-identifying, and a LAN/remote Studio + // (`HYPERFRAMES_PREVIEW_HOST=0.0.0.0`) needs it to stay in agreement with + // the CLI. See buildStudioHeadScriptsForHost. + const headScript = buildStudioHeadScriptsForHost(buildRuntimeEnvScript(), c.req.header("host")); if (headScript) { html = html.replace("", `${headScript}`); } diff --git a/packages/cli/src/server/telemetryIdentity.test.ts b/packages/cli/src/server/telemetryIdentity.test.ts index 9f779841fb..169f9ceb27 100644 --- a/packages/cli/src/server/telemetryIdentity.test.ts +++ b/packages/cli/src/server/telemetryIdentity.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi, beforeEach } from "vitest"; +import { afterEach, describe, expect, it, vi, beforeEach } from "vitest"; // CLI → Studio telemetry identity seeding (Layer 1). Verifies the server only // hands the browser a distinct id when CLI telemetry is enabled, and passes @@ -6,6 +6,9 @@ import { describe, expect, it, vi, beforeEach } from "vitest"; const shouldTrack = vi.fn(); const readConfig = vi.fn(); +// Pinned rather than using the real registry, so these string assertions +// don't move every time a canary is added, ramped, or retired. +const canaryDecisions = vi.fn<() => Record>(); vi.mock("../telemetry/client.js", () => ({ shouldTrack: (...args: unknown[]) => shouldTrack(...args), @@ -13,14 +16,25 @@ vi.mock("../telemetry/client.js", () => ({ vi.mock("../telemetry/config.js", () => ({ readConfig: (...args: unknown[]) => readConfig(...args), })); +vi.mock("../telemetry/canary.js", () => ({ + canaryDecisionsForStudio: () => canaryDecisions(), +})); -const { resolveCliTelemetryDistinctId, buildCliIdentityScript, buildStudioHeadScripts } = - await import("./telemetryIdentity.js"); +const { + resolveCliTelemetryDistinctId, + buildCliIdentityScript, + buildStudioHeadScripts, + isLoopbackHost, + buildStudioHeadScriptsForHost, + identityAllowed, +} = await import("./telemetryIdentity.js"); describe("resolveCliTelemetryDistinctId", () => { beforeEach(() => { shouldTrack.mockReset(); readConfig.mockReset(); + canaryDecisions.mockReset(); + canaryDecisions.mockReturnValue({}); }); it("returns the CLI anonymousId when telemetry is enabled", () => { @@ -56,6 +70,8 @@ describe("buildCliIdentityScript", () => { beforeEach(() => { shouldTrack.mockReset(); readConfig.mockReset(); + canaryDecisions.mockReset(); + canaryDecisions.mockReturnValue({}); }); it("emits a script that sets window.__HF_CLI_DISTINCT_ID when telemetry is on", () => { @@ -66,11 +82,68 @@ describe("buildCliIdentityScript", () => { ); }); - it("emits an empty string when telemetry is disabled (nothing to seed)", () => { + it("also seeds window.__HF_CLI_BUCKET_SEED when the config carries a bucket seed", () => { + shouldTrack.mockReturnValue(true); + readConfig.mockReturnValue({ anonymousId: "machine-uuid", bucketSeed: "seed-uuid" }); + expect(buildCliIdentityScript()).toBe( + '', + ); + }); + + it("emits an empty string when telemetry is off and there are no canaries", () => { shouldTrack.mockReturnValue(false); expect(buildCliIdentityScript()).toBe(""); }); + // The cross-surface fix: with telemetry off the CLI resolves every canary + // to telemetry_opt_out, and Studio cannot see that from its own separate + // localStorage flag. Publishing the DECISIONS (not the identity) is what + // stops Studio evaluating independently and enrolling anyway. + it("still publishes canary decisions when telemetry is off, but no identity", () => { + shouldTrack.mockReturnValue(false); + canaryDecisions.mockReturnValue({ "de-parallel-router": { enabled: false, forced: false } }); + const script = buildCliIdentityScript(); + expect(script).toBe( + "', + ); + expect(script).not.toContain("__HF_CLI_DISTINCT_ID"); + expect(script).not.toContain("__HF_CLI_BUCKET_SEED"); + }); + + it("publishes decisions alongside the identity when telemetry is on", () => { + shouldTrack.mockReturnValue(true); + readConfig.mockReturnValue({ anonymousId: "machine-uuid", bucketSeed: "seed-uuid" }); + canaryDecisions.mockReturnValue({ "de-parallel-router": { enabled: true, forced: true } }); + expect(buildCliIdentityScript()).toBe( + '', + ); + }); + + it("escapes a canary name that tries to close the script tag", () => { + shouldTrack.mockReturnValue(false); + canaryDecisions.mockReturnValue({ + "', + ); + }); + it("JSON-encodes the id so it can't break out of the script literal", () => { shouldTrack.mockReturnValue(true); readConfig.mockReturnValue({ anonymousId: ""; @@ -97,8 +172,129 @@ describe("buildStudioHeadScripts", () => { expect(head.indexOf("__HF_CLI_DISTINCT_ID")).toBeLessThan(head.indexOf("__HF_STUDIO_ENV__")); }); - it("returns just the env script when identity is suppressed (telemetry off)", () => { + it("returns just the env script when there is no identity and no canary", () => { shouldTrack.mockReturnValue(false); expect(buildStudioHeadScripts(ENV_SCRIPT)).toBe(ENV_SCRIPT); }); }); + +describe("isLoopbackHost (DNS-rebinding guard on the identity endpoint)", () => { + it.each([ + "localhost", + "localhost:5173", + "127.0.0.1", + "127.0.0.1:5173", + "127.1.2.3", + "[::1]", + "[::1]:5173", + "LOCALHOST:5173", + ])("accepts the loopback host %s", (host) => { + expect(isLoopbackHost(host)).toBe(true); + }); + + it.each([ + undefined, + "", + // The rebinding case: an attacker hostname resolving to 127.0.0.1 still + // arrives with ITS name in Host, which is what makes this catchable. + "evil.example.com", + "evil.example.com:5173", + "127.0.0.1.evil.com", + "notlocalhost", + "localhost.evil.com", + "192.168.1.10", + "0.0.0.0", + ])("rejects %s", (host) => { + expect(isLoopbackHost(host)).toBe(false); + }); +}); + +describe("buildStudioHeadScriptsForHost — Host split", () => { + const ENV = ""; + + beforeEach(() => { + shouldTrack.mockReset(); + readConfig.mockReset(); + canaryDecisions.mockReset(); + shouldTrack.mockReturnValue(true); + readConfig.mockReturnValue({ anonymousId: "machine-uuid", bucketSeed: "seed-uuid" }); + canaryDecisions.mockReturnValue({ "de-parallel-router": { enabled: true, forced: false } }); + }); + + it("publishes identity and decisions on a loopback Host", () => { + const head = buildStudioHeadScriptsForHost(ENV, "127.0.0.1:5173"); + expect(head).toContain("__HF_CLI_DISTINCT_ID"); + expect(head).toContain("__HF_CLI_BUCKET_SEED"); + expect(head).toContain("__HF_CLI_CANARY_DECISIONS"); + }); + + // DNS rebinding: identity must not be readable from a hostile origin. + it("withholds identity and seed from a hostile Host", () => { + const head = buildStudioHeadScriptsForHost(ENV, "evil.example.com"); + expect(head).not.toContain("__HF_CLI_DISTINCT_ID"); + expect(head).not.toContain("__HF_CLI_BUCKET_SEED"); + }); + + // ...but the decisions map is NOT identifying, and withholding it would send + // a supported LAN preview (HYPERFRAMES_PREVIEW_HOST=0.0.0.0) back to + // re-deriving locally and disagreeing with the CLI. + it.each(["evil.example.com", "192.168.1.10:5173", "my-dev-box.local:5173", undefined])( + "still publishes canary decisions for non-loopback Host %s", + (host) => { + const head = buildStudioHeadScriptsForHost(ENV, host); + expect(head).toContain("__HF_CLI_CANARY_DECISIONS"); + expect(head).not.toContain("__HF_CLI_DISTINCT_ID"); + }, + ); + + it("always keeps the env script, whatever the Host", () => { + expect(buildStudioHeadScriptsForHost(ENV, "evil.example.com")).toContain("__HF_STUDIO_ENV__"); + expect(buildStudioHeadScriptsForHost(ENV, "localhost")).toContain("__HF_STUDIO_ENV__"); + }); +}); + +describe("identityAllowed — loopback-bound vs explicitly LAN-bound", () => { + const original = process.env["HYPERFRAMES_PREVIEW_HOST"]; + + afterEach(() => { + if (original === undefined) delete process.env["HYPERFRAMES_PREVIEW_HOST"]; + else process.env["HYPERFRAMES_PREVIEW_HOST"] = original; + }); + + describe("loopback-bound (the default)", () => { + beforeEach(() => { + delete process.env["HYPERFRAMES_PREVIEW_HOST"]; + }); + + it.each(["localhost:5173", "127.0.0.1", "[::1]:3000"])("allows %s", (host) => { + expect(identityAllowed(host)).toBe(true); + }); + + // A rebinding page cannot forge Host, so it arrives carrying its own name. + it.each(["evil.example.com", "127.0.0.1.evil.com", "192.168.1.10:3000", undefined])( + "refuses %s", + (host) => { + expect(identityAllowed(host)).toBe(false); + }, + ); + }); + + describe("explicitly LAN-bound", () => { + beforeEach(() => { + process.env["HYPERFRAMES_PREVIEW_HOST"] = "0.0.0.0"; + }); + + // The mode this regressed: browsing your own LAN-exposed Studio lost the + // CLI stitch entirely, so the same human became two PostHog persons. + it.each(["0.0.0.0:3000", "192.168.1.10:3000", "my-dev-box.local:3000"])( + "allows %s once the operator opted into LAN exposure", + (host) => { + expect(identityAllowed(host)).toBe(true); + }, + ); + + it("still allows loopback in that mode", () => { + expect(identityAllowed("localhost:3000")).toBe(true); + }); + }); +}); diff --git a/packages/cli/src/server/telemetryIdentity.ts b/packages/cli/src/server/telemetryIdentity.ts index f5d4d36df0..a518a87083 100644 --- a/packages/cli/src/server/telemetryIdentity.ts +++ b/packages/cli/src/server/telemetryIdentity.ts @@ -18,6 +18,7 @@ import { readConfig } from "../telemetry/config.js"; import { shouldTrack as telemetryShouldTrack } from "../telemetry/client.js"; +import { canaryDecisionsForStudio, type CliCanaryDecision } from "../telemetry/canary.js"; /** * The CLI's anonymous distinct id to hand to Studio, or null when CLI telemetry @@ -34,22 +35,124 @@ export function resolveCliTelemetryDistinctId(): string | null { } } +/** + * The CLI's canary bucket seed to hand to Studio, or null. Injected alongside + * the distinct id so a CLI-launched Studio buckets canaries on the SAME unit + * as the CLI — without it the two surfaces would agree only while the seed + * still equals whatever Studio falls back to, and a rollout spanning render + * and editor would split one user across cohorts. Same telemetry gate as the + * distinct id: seeding is part of the identity stitch, not a separate channel. + */ +function resolveCliBucketSeed(): string | null { + try { + if (!telemetryShouldTrack()) return null; + const seed = readConfig().bucketSeed; + return typeof seed === "string" && seed.length > 0 ? seed : null; + } catch { + return null; + } +} + +/** + * Is this request's `Host` a loopback name the studio server could have been + * reached on directly? + * + * A bare `[::1]`/`localhost`/dotted-quad check rather than a full parse: the + * port is irrelevant (any port on loopback is us), and anything exotic enough + * to miss here should be refused rather than guessed at. + * + * Scope, stated precisely: this is a **DNS-rebinding mitigation for browsers**, + * not access control. A browser sets `Host` from the URL it was given, so a + * page that rebinds its own hostname to 127.0.0.1 arrives carrying that + * hostname and is refused. A non-browser client sets `Host` to whatever it + * likes, so this stops nothing there — but on a loopback-bound server such a + * client is already local, and on a LAN-bound one it can read the project + * files through the unauthenticated studio API anyway. See `identityAllowed`. + */ +export function isLoopbackHost(host: string | undefined): boolean { + if (!host) return false; + // Strip the port. IPv6 literals are bracketed (`[::1]:1234`), so take the + // bracketed part when present and only split on ":" otherwise. + const bracketed = /^\[([^\]]+)\]/.exec(host); + const hostname = (bracketed ? bracketed[1] : host.split(":")[0])?.toLowerCase() ?? ""; + if (hostname === "localhost" || hostname === "::1" || hostname === "0:0:0:0:0:0:0:1") return true; + // 127.0.0.0/8 — the whole loopback block, not just 127.0.0.1. + return /^127\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.test(hostname); +} + +// JSON.stringify does not escape "<" or "/". Escaping both means no +// "" (or " or open a new tag. (The values are +// randomUUID()s, so this is belt-and-suspenders.) +function encodeInlineScriptValue(value: string): string { + return JSON.stringify(value).replace(/ | null { + try { + const decisions = canaryDecisionsForStudio(); + return Object.keys(decisions).length > 0 ? decisions : null; + } catch { + return null; + } +} + /** * `" (or " or open a new tag. - const encoded = JSON.stringify(cliId).replace(/window.__HF_CLI_DISTINCT_ID=${encoded};`; +export function buildCliIdentityScript(options: { includeIdentity?: boolean } = {}): string { + const { includeIdentity = true } = options; + const parts: string[] = []; + + // Identity is the only part gated on a trusted Host. The decisions map below + // is not identifying, and withholding it would push a LAN/remote Studio + // (`HYPERFRAMES_PREVIEW_HOST=0.0.0.0`, an explicitly supported mode) back to + // re-deriving locally — reopening exactly the CLI/Studio disagreement this + // whole mechanism exists to close. + const cliId = includeIdentity ? resolveCliTelemetryDistinctId() : null; + if (cliId) { + parts.push(`window.__HF_CLI_DISTINCT_ID=${encodeInlineScriptValue(cliId)};`); + const seed = resolveCliBucketSeed(); + if (seed) parts.push(`window.__HF_CLI_BUCKET_SEED=${encodeInlineScriptValue(seed)};`); + } + + // Emitted even when telemetry is OFF and the identity block above is empty — + // that is the case it exists for. With telemetry off the CLI resolves every + // canary to `telemetry_opt_out`, but Studio's opt-out is a separate + // localStorage flag it cannot see, so left to itself Studio would evaluate + // normally and could enrol on a render the CLI had already excluded. Same + // for an `HF_CANARY_*` override, which never crosses into the browser. + // + // Safe to publish unconditionally: these are booleans about features, not + // identity, and strictly less than the seed they replace as Studio's input. + const decisions = resolveCliCanaryDecisions(); + if (decisions !== null) { + parts.push(`window.__HF_CLI_CANARY_DECISIONS=${encodeInlineScriptJson(decisions)};`); + } + + return parts.length === 0 ? "" : ``; } /** @@ -60,6 +163,43 @@ export function buildCliIdentityScript(): string { * ordering in one pure, tested function guards against a future `` inject * silently landing ahead of the identity script and reintroducing a boot race. */ -export function buildStudioHeadScripts(envScript: string): string { - return `${buildCliIdentityScript()}${envScript}`; +export function buildStudioHeadScripts( + envScript: string, + options: { includeIdentity?: boolean } = {}, +): string { + return `${buildCliIdentityScript(options)}${envScript}`; +} + +/** + * The `` scripts for a request, given its `Host`. + * + * The Host split lives here rather than in the route so it is testable + * without a Studio bundle on disk — the route's own test can only reach the + * injection branch when `packages/studio/dist` happens to be built, which is + * true locally and false in the CI test lane. + */ +/** + * May this request receive the CLI's identity (distinct id + bucket seed)? + * + * Two regimes, because the server binds loopback by DEFAULT and exposes the + * LAN only when an operator sets `HYPERFRAMES_PREVIEW_HOST` (portUtils.ts, + * F-001): + * + * - **Loopback-bound (default).** Anything reaching us came via loopback, so + * the only interesting attacker is a rebinding browser page — which the + * Host check catches, because a browser cannot forge `Host`. + * - **Explicitly LAN-bound.** The operator opted into exposing this server, + * and the Host header is trivially forgeable by any non-browser client, so + * the check buys nothing. Withholding identity there only broke the + * CLI-to-Studio stitch for the supported mode: the user browses + * `http://0.0.0.0:3000` or the machine's LAN IP, `isLoopbackHost` says no, + * and Studio mints a second anonymous person for the same human. + */ +export function identityAllowed(host: string | undefined): boolean { + const lanBound = (process.env["HYPERFRAMES_PREVIEW_HOST"] ?? "").trim() !== ""; + return lanBound || isLoopbackHost(host); +} + +export function buildStudioHeadScriptsForHost(envScript: string, host: string | undefined): string { + return buildStudioHeadScripts(envScript, { includeIdentity: identityAllowed(host) }); } diff --git a/packages/cli/src/telemetry/canary.test.ts b/packages/cli/src/telemetry/canary.test.ts new file mode 100644 index 0000000000..4771c0c145 --- /dev/null +++ b/packages/cli/src/telemetry/canary.test.ts @@ -0,0 +1,237 @@ +import { describe, expect, it, vi, beforeEach } from "vitest"; + +const configState: { + anonymousId: string; + bucketSeed: string | undefined; + telemetryEnabled: boolean; +} = { + anonymousId: "db0c1f4a-b95e-4c35-90c6-1a15bd76f717", + bucketSeed: undefined, + telemetryEnabled: true, +}; +const systemState = { is_ci: false }; +// null = no runtime opt-out in force; a string names the source (env var, +// dev build, ...) exactly as policy.ts reports it. +const policyState: { runtimeOverride: string | null } = { runtimeOverride: null }; + +vi.mock("./config.js", () => ({ + readConfig: () => ({ + anonymousId: configState.anonymousId, + bucketSeed: configState.bucketSeed, + telemetryEnabled: configState.telemetryEnabled, + }), +})); +vi.mock("./system.js", () => ({ + getSystemMeta: () => ({ is_ci: systemState.is_ci }), +})); +vi.mock("./policy.js", () => ({ + telemetryRuntimeOverride: () => policyState.runtimeOverride, +})); + +// 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/canary-registry", async () => { + const actual = await vi.importActual( + "@hyperframes/core/canary-registry", + ); + return { + ...actual, + CANARIES: [ + { + name: "test-alpha", + percentage: 100, + description: "always on", + owner: "t", + sunsetAfter: "2099-01-01", + }, + { + // A PARTIAL percentage. `exclude` and the unit-id check only apply + // below 100 — at 100 the guard is about to be deleted, so everyone + // must already be on it — so exclusion behaviour cannot be expressed + // against test-alpha. + name: "test-gamma", + percentage: 50, + description: "partial rollout", + owner: "t", + sunsetAfter: "2099-01-01", + }, + { + name: "test-beta", + percentage: 0, + description: "always off", + owner: "t", + sunsetAfter: "2099-01-01", + }, + ], + findCanary: (n: string) => + [ + { + name: "test-alpha", + percentage: 100, + description: "", + owner: "t", + sunsetAfter: "2099-01-01", + }, + { + name: "test-gamma", + percentage: 50, + description: "", + owner: "t", + sunsetAfter: "2099-01-01", + }, + { + name: "test-beta", + percentage: 0, + description: "", + owner: "t", + sunsetAfter: "2099-01-01", + }, + ].find((c) => c.name === n), + }; +}); + +const { isCanaryEnabled, resolveCanary, canaryEventProperties, __resetCanaryCacheForTests } = + await import("./canary.js"); + +beforeEach(() => { + __resetCanaryCacheForTests(); + configState.anonymousId = "db0c1f4a-b95e-4c35-90c6-1a15bd76f717"; + configState.bucketSeed = undefined; + configState.telemetryEnabled = true; + systemState.is_ci = false; + policyState.runtimeOverride = null; + delete process.env.HF_CANARY_TEST_ALPHA; + delete process.env.HF_CANARY_TEST_BETA; + delete process.env.HF_CANARY_TEST_GAMMA; +}); + +describe("telemetry opt-out is canary opt-out", () => { + it("does not enrol when the persisted preference is off", () => { + configState.telemetryEnabled = false; + // test-alpha is at 100% — it would be on for everyone otherwise. + expect(resolveCanary("test-alpha")).toEqual({ + enabled: false, + reason: "telemetry_opt_out", + }); + }); + + it.each(["HYPERFRAMES_NO_TELEMETRY", "DO_NOT_TRACK", "dev_mode"])( + "does not enrol under the %s runtime override, even with the preference on", + (source) => { + configState.telemetryEnabled = true; + policyState.runtimeOverride = source; + expect(resolveCanary("test-alpha").reason).toBe("telemetry_opt_out"); + }, + ); + + it("never buckets an opted-out install — no cohort is assigned at all", () => { + configState.telemetryEnabled = false; + // A bucket number would mean we hashed them into a slice anyway. + expect(resolveCanary("test-alpha").bucket).toBeUndefined(); + }); + + it("still honours an explicit override — the documented way to test with telemetry off", () => { + configState.telemetryEnabled = false; + process.env.HF_CANARY_TEST_BETA = "on"; + expect(resolveCanary("test-beta")).toEqual({ enabled: true, reason: "forced_on" }); + }); + + it("reports every canary as false to PostHog shape when opted out", () => { + configState.telemetryEnabled = false; + expect(canaryEventProperties()).toEqual({ + "$feature/canary-test-alpha": "false", + "$feature/canary-test-gamma": "false", + "$feature/canary-test-beta": "false", + }); + }); +}); + +describe("bucketing unit", () => { + it("buckets on the bucketSeed when present — the unit that survives config wipes", async () => { + const { evaluateCanary } = await import("@hyperframes/core/canary"); + configState.bucketSeed = "5f1c9d2e-0000-4000-8000-aaaaaaaaaaaa"; + const viaBinding = resolveCanary("test-gamma").bucket; + // 50, not 100: at 100 evaluateCanary short-circuits before bucketing and + // reports no bucket at all, which would make this comparison vacuous. + const bySeed = evaluateCanary({ + feature: "test-gamma", + unitId: configState.bucketSeed, + percentage: 50, + }).bucket; + const byId = evaluateCanary({ + feature: "test-gamma", + unitId: configState.anonymousId, + percentage: 50, + }).bucket; + expect(viaBinding).toBe(bySeed); + // Only meaningful if the two units actually bucket differently. + expect(bySeed).not.toBe(byId); + }); + + it("falls back to the anonymousId when no seed exists (failed legacy backfill)", async () => { + const { evaluateCanary } = await import("@hyperframes/core/canary"); + const viaBinding = resolveCanary("test-gamma").bucket; + const byId = evaluateCanary({ + feature: "test-gamma", + unitId: configState.anonymousId, + percentage: 50, + }).bucket; + expect(viaBinding).toBe(byId); + // Both undefined would satisfy toBe — assert a bucket was actually computed. + expect(viaBinding).toEqual(expect.any(Number)); + }); +}); + +describe("CLI canary binding", () => { + it("reads the percentage from the registry", () => { + expect(isCanaryEnabled("test-alpha")).toBe(true); + expect(isCanaryEnabled("test-beta")).toBe(false); + }); + + it("an unregistered name is off, not a throw — a typo must not break a render", () => { + expect(isCanaryEnabled("does-not-exist")).toBe(false); + expect(resolveCanary("does-not-exist").reason).toBe("out_of_cohort"); + }); + + it("HF_CANARY_ overrides the registry in both directions", () => { + process.env.HF_CANARY_TEST_ALPHA = "off"; + process.env.HF_CANARY_TEST_BETA = "on"; + expect(resolveCanary("test-alpha")).toMatchObject({ enabled: false, reason: "forced_off" }); + expect(resolveCanary("test-beta")).toMatchObject({ enabled: true, reason: "forced_on" }); + }); + + it("excludes CI from percentage enrolment, but an override still reaches it", () => { + systemState.is_ci = true; + expect(resolveCanary("test-gamma")).toMatchObject({ enabled: false, reason: "excluded" }); + + __resetCanaryCacheForTests(); + process.env.HF_CANARY_TEST_GAMMA = "on"; + expect(resolveCanary("test-gamma")).toMatchObject({ enabled: true, reason: "forced_on" }); + }); + + it("fails closed when the install has no anonymousId", () => { + configState.anonymousId = ""; + expect(resolveCanary("test-gamma")).toMatchObject({ enabled: false, reason: "no_unit_id" }); + }); + + it("memoizes so a decision cannot change mid-process", () => { + expect(isCanaryEnabled("test-beta")).toBe(false); + // A late env change must NOT flip a render that already started. + process.env.HF_CANARY_TEST_BETA = "on"; + expect(isCanaryEnabled("test-beta")).toBe(false); + __resetCanaryCacheForTests(); + expect(isCanaryEnabled("test-beta")).toBe(true); + }); + + it("emits PostHog flag-shaped properties for every registered canary", () => { + expect(canaryEventProperties()).toEqual({ + "$feature/canary-test-alpha": "true", + "$feature/canary-test-gamma": expect.stringMatching(/^(true|false)$/), + "$feature/canary-test-beta": "false", + }); + + __resetCanaryCacheForTests(); + process.env.HF_CANARY_TEST_ALPHA = "off"; + expect(canaryEventProperties()["$feature/canary-test-alpha"]).toBe("false"); + }); +}); diff --git a/packages/cli/src/telemetry/canary.ts b/packages/cli/src/telemetry/canary.ts new file mode 100644 index 0000000000..f14de2e5b7 --- /dev/null +++ b/packages/cli/src/telemetry/canary.ts @@ -0,0 +1,192 @@ +/** + * CLI binding for the canary registry. + * + * `@hyperframes/core` owns the decision (pure, browser-safe, caller supplies + * everything). This file supplies the three things only the CLI knows: the + * install's stable id, the env override, and whether we're on CI. + * + * Using one, from anywhere in the CLI / producer call path: + * + * ```ts + * import { isCanaryEnabled } from "../telemetry/canary.js"; + * if (isCanaryEnabled("de-parallel-router")) { ...ramped path... } + * ``` + * + * That is the whole API. Percentage lives in the registry, not at the call + * site, so ramping is a one-line edit in a patch release and never touches + * the feature's own code. + */ + +// 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 { + canaryFeatureProperties, + evaluateCanary, + parseCanaryOverride, + type CanaryDecision, +} from "@hyperframes/core/canary"; +import { CANARIES, canaryEnvVar, findCanary } from "@hyperframes/core/canary-registry"; +import { readConfig } from "./config.js"; +import { telemetryRuntimeOverride } from "./policy.js"; +import { getSystemMeta } from "./system.js"; + +/** + * Opting out of telemetry opts you out of canaries. + * + * A canary is a measured rollout: we enrol a slice precisely so we can compare + * it against everyone else. An install that sends nothing can't be compared, + * so enrolling it buys no signal — it only changes that user's code path, on + * an experimental feature, without their knowledge and with no way for us to + * see the result. That is the wrong side of an opt-out. + * + * Deliberately mirrors `shouldTrack()` rather than importing it: client.ts + * already imports this module for `canaryEventProperties`, so depending on it + * here would be a cycle. Both read the same two inputs (`policy.ts`'s runtime + * override, then the persisted preference), so they cannot disagree. + */ +function telemetryActive(): boolean { + if (telemetryRuntimeOverride() !== null) return false; + return readConfig().telemetryEnabled; +} + +/** + * Decisions are memoized per process: a `--batch` run asks the same question + * once per row, and a canary must not change its mind mid-process — a render + * that starts enrolled has to finish enrolled, and its telemetry has to agree + * with what actually ran. + */ +const decisions = new Map(); + +// The memo is scoped to a telemetry posture, not to the process. Within one +// render nothing may change (a render that starts enrolled must finish +// enrolled), but `hyperframes preview` is a long-lived server that re-serves +// decisions on every page load — and it used to keep serving pre-opt-out +// decisions for hours after `hyperframes telemetry disable`, which is exactly +// what the docs promise cannot happen. +let decisionsTelemetryPosture: boolean | undefined; + +/** Test-only: drop memoized decisions so cases don't leak into each other. */ +export function __resetCanaryCacheForTests(): void { + decisions.clear(); + decisionsTelemetryPosture = undefined; +} + +/** The uncached decision. Split out so `resolveCanary` is purely the memo. */ +function decideCanary(name: string): CanaryDecision { + const definition = findCanary(name); + if (!definition) return { enabled: false, reason: "out_of_cohort" }; + + const override = parseCanaryOverride(process.env[canaryEnvVar(definition.name)]); + + // Resolved ahead of evaluate so an opted-out install is never bucketed at + // all. An explicit HF_CANARY_* override still wins: that is a deliberate + // local choice (support session, bisect, developer testing), not silent + // enrolment, and it stays the way to exercise a canary with telemetry off. + if (override === undefined && !telemetryActive()) { + return { enabled: false, reason: "telemetry_opt_out" }; + } + + const config = readConfig(); + return evaluateCanary({ + feature: definition.name, + // The bucket seed, NOT the anonymousId: the seed is inherited across + // config.json re-mints via the install-state file, so cohorts hold when + // the telemetry id re-rolls. Both files live in ~/.hyperframes, so + // deleting that directory clears the cohort too — intentional. Fallback + // covers only a failed backfill write on a legacy config. + unitId: config.bucketSeed ?? config.anonymousId, + percentage: definition.percentage, + override, + // CI installs regenerate their config per run, so their ids are + // ephemeral — they would hop cohorts between runs, adding noise to the + // rollout signal while saying nothing about real users. An explicit + // override still gets through, which is how you test a canary in CI. + exclude: getSystemMeta().is_ci, + }); +} + +/** + * Full decision for a registered canary, including the reason — use this when + * you want to record WHY, not just whether. + * + * An unregistered name resolves to off rather than throwing: a canary is a + * rollout control, and a typo in one must never take down a render. + */ +export function resolveCanary(name: string): CanaryDecision { + const posture = telemetryActive(); + if (decisionsTelemetryPosture !== posture) { + decisions.clear(); + decisionsTelemetryPosture = posture; + } + const cached = decisions.get(name); + if (cached) return cached; + + const decision = decideCanary(name); + decisions.set(name, decision); + return decision; +} + +/** Is this canary on for this install? The everyday call. */ +export function isCanaryEnabled(name: string): boolean { + return resolveCanary(name).enabled; +} + +/** + * One canary's outcome as handed to Studio: the answer, plus whether it came + * from an explicit override rather than a percentage roll. + */ +export interface CliCanaryDecision { + enabled: boolean; + forced: boolean; +} + +/** + * Every registered canary's resolved on/off for this process, for handing to + * a CLI-launched Studio. + * + * Studio adopts these wholesale instead of re-deriving, because re-deriving + * cannot agree in three cases: + * + * - **Telemetry off.** The CLI resolves `telemetry_opt_out`; Studio's own + * opt-out is a separate localStorage flag on a different machine-level + * switch, so it would evaluate normally and could enrol. + * - **`HF_CANARY_*` override.** Env vars do not cross into the browser at + * all — Studio only reads its URL param / sessionStorage — so a support + * session forcing a canary on got the CLI forced and Studio guessing. + * - **No seed injected.** Whenever the seed is withheld Studio falls back + * to a different unit id, i.e. a different bucket. + * + * Shipping the decision rather than the inputs makes divergence structurally + * impossible: one evaluation, two surfaces. It is also strictly less to + * expose — booleans about features, not the seed the buckets derive from. + */ +export function canaryDecisionsForStudio(): Record { + const out: Record = {}; + for (const canary of CANARIES) { + const { enabled, reason } = resolveCanary(canary.name); + out[canary.name] = { + enabled, + // Provenance, not decoration. Studio has its own telemetry opt-out that + // the CLI cannot see, and it must be able to refuse ordinary cohort + // enrolment while still honouring a deliberate `HF_CANARY_*` override. + // A bare boolean cannot express that difference, so Studio would have + // had to either ignore its own opt-out or drop the override. + forced: reason === "forced_on" || reason === "forced_off", + }; + } + return out; +} + +/** + * Canary assignments as PostHog flag properties — `$feature/canary-` + * set to `"true"` / `"false"` for every registered canary. Spread onto every + * event so any metric can be broken down by cohort using PostHog's native + * flag tooling, with nothing configured server-side. See + * `canaryFeatureProperties` for why non-enrolled canaries are emitted too. + */ +export function canaryEventProperties(): Record { + return canaryFeatureProperties( + CANARIES.map((c) => ({ name: c.name, enabled: resolveCanary(c.name).enabled })), + ); +} diff --git a/packages/cli/src/telemetry/client.test.ts b/packages/cli/src/telemetry/client.test.ts index 63d26b6fbf..149bf14840 100644 --- a/packages/cli/src/telemetry/client.test.ts +++ b/packages/cli/src/telemetry/client.test.ts @@ -17,6 +17,17 @@ vi.mock("../utils/env.js", () => ({ isDevMode: () => false, })); +// Canary enrolment is registry-driven and will change as rollouts ramp; stub +// it so this asserts the WIRING (does every event carry the cohort?) rather +// than whichever canaries happen to be live today. +const canaryProps = vi.fn<() => Record>(() => ({ + "$feature/canary-feat-x": "true", + "$feature/canary-feat-y": "false", +})); +vi.mock("./canary.js", () => ({ + canaryEventProperties: () => canaryProps(), +})); + // Intercept the exit-time child process so flushSync delivery is assertable. const spawnMock = vi.fn(() => ({ unref: vi.fn() })); vi.mock("node:child_process", () => ({ @@ -33,6 +44,16 @@ function sentBatch(fetchMock: ReturnType, call = 0): Batch { return JSON.parse(init.body).batch; } +/** Properties of the first event in the first delivered batch. */ +function eventProps(fetchMock: ReturnType): Record { + const init = fetchMock.mock.calls[0]?.[1] as { body: string } | undefined; + if (!init) throw new Error("expected a fetch call to have been made"); + const parsed = JSON.parse(init.body) as { + batch: Array<{ properties?: Record }>; + }; + return parsed.batch[0]?.properties ?? {}; +} + describe("telemetry queue delivery", () => { beforeEach(async () => { vi.unstubAllGlobals(); @@ -132,3 +153,36 @@ describe("telemetry queue delivery", () => { expect(fetchMock).not.toHaveBeenCalled(); }); }); + +describe("canary cohort on every event", () => { + it("attaches canary assignments as PostHog flag properties", async () => { + canaryProps.mockReturnValue({ + "$feature/canary-feat-x": "true", + "$feature/canary-feat-y": "false", + }); + const fetchMock = vi.fn(async () => ({ ok: true, status: 200 }) as Response); + vi.stubGlobal("fetch", fetchMock); + + trackEvent("cli_command", { command: "render" }); + await flush(); + + const props = eventProps(fetchMock); + // Enrolled AND control are both emitted — absent would mean "this build + // predates the canary", a different fact from "not enrolled". + expect(props["$feature/canary-feat-x"]).toBe("true"); + expect(props["$feature/canary-feat-y"]).toBe("false"); + }); + + it("adds no canary properties when the registry is empty", async () => { + canaryProps.mockReturnValue({}); + const fetchMock = vi.fn(async () => ({ ok: true, status: 200 }) as Response); + vi.stubGlobal("fetch", fetchMock); + + trackEvent("cli_command", { command: "render" }); + await flush(); + + expect(Object.keys(eventProps(fetchMock)).some((k) => k.startsWith("$feature/canary-"))).toBe( + false, + ); + }); +}); diff --git a/packages/cli/src/telemetry/client.ts b/packages/cli/src/telemetry/client.ts index 24692f8679..1c9a577c5d 100644 --- a/packages/cli/src/telemetry/client.ts +++ b/packages/cli/src/telemetry/client.ts @@ -3,6 +3,7 @@ import { VERSION } from "../version.js"; import { c } from "../ui/colors.js"; import { diag } from "../ui/diagnostics.js"; import { getSystemMeta } from "./system.js"; +import { canaryEventProperties } from "./canary.js"; import { enqueue, type EventProperties } from "./transport.js"; import { telemetryRuntimeOverride } from "./policy.js"; @@ -75,10 +76,21 @@ export function trackEvent( term_program: sys.term_program ?? undefined, // Did this install's mint find a previous install's state marker? // The fleet-wide rate of `true` IS the recoverable-churn fraction — - // the share of "new" ids that are really a config wipe on a machine + // the share of "new" ids that are really a config re-mint on a machine // we already knew. Absent (not false) when the config predates the // marker. Resolved after the shouldTrack guard. install_predecessor_found: readConfig().predecessorFound, + // Splits the `true` share above: a machine we knew but whose record we + // could not read. Without it a partial disk write is indistinguishable + // from a genuinely fresh install. Absent in the normal case. + install_state_file_corrupt: readConfig().stateFileCorrupt, + // Canary assignments as `$feature/canary-` — PostHog's native flag + // property shape, so breakdowns and experiment analysis work on a canary + // with nothing configured server-side. On EVERY event, not just renders: + // a staged rollout is only as good as the ability to split any metric by + // cohort. Resolved after the shouldTrack guard, so opted-out installs + // never pay for it. See telemetry/canary.ts. + ...canaryEventProperties(), agent_env_hints: sys.agent_env_hints ?? undefined, }, distinctId, diff --git a/packages/cli/src/telemetry/config.test.ts b/packages/cli/src/telemetry/config.test.ts index 571ef736a5..a5e7674409 100644 --- a/packages/cli/src/telemetry/config.test.ts +++ b/packages/cli/src/telemetry/config.test.ts @@ -36,6 +36,19 @@ vi.mock("node:fs", () => ({ }), })); +// The backfill warning is suppressed under a telemetry runtime override (a +// dev build counts as one, which vitest is), so the posture is controlled +// explicitly rather than inherited from the test environment. +const policyState = { runtimeOverride: null as string | null }; +vi.mock("./policy.js", () => ({ + telemetryRuntimeOverride: () => policyState.runtimeOverride, +})); + +// Derived here rather than exported from config.ts: the pre-move path is +// frozen history, so pinning the literal is the point — an export would just +// let a rename pass silently, and it has no non-test consumer. +const LEGACY_STATE_PATH = join(homedir(), ".local", "state", "hyperframes", "install-state.json"); + describe("config.ts — readConfig / readConfigFresh / writeConfig (real module, faked fs)", () => { let readConfig: typeof import("./config.js").readConfig; let readConfigFresh: typeof import("./config.js").readConfigFresh; @@ -143,11 +156,6 @@ describe("install-state rollover (breaker survives a config re-mint)", () => { let CONFIG_PATH: typeof import("./config.js").CONFIG_PATH; let STATE_PATH: typeof import("./config.js").STATE_PATH; - // Derived here rather than exported from config.ts: the pre-move path is - // frozen history, so pinning the literal is the point — an export would - // just let a rename pass silently, and it has no non-test consumer. - const LEGACY_STATE_PATH = join(homedir(), ".local", "state", "hyperframes", "install-state.json"); - beforeEach(async () => { fsState.files.clear(); vi.resetModules(); @@ -204,11 +212,18 @@ describe("install-state rollover (breaker survives a config re-mint)", () => { expect(readConfigFresh().deParallelRouterTrialFired).toBe(true); }); - it("the state file holds no identity — only the marker timestamp and breaker fact", () => { + it("the state file holds no telemetry identity — marker, breaker fact, bucket seed only", () => { const config = readConfig(); config.deParallelRouterTrialFired = true; writeConfig(config); - expect(Object.keys(stateFile()).sort()).toEqual(["deParallelRouterTrialFired", "markerAt"]); + expect(Object.keys(stateFile()).sort()).toEqual([ + "bucketSeed", + "deParallelRouterTrialFired", + "markerAt", + ]); + // The seed is a separate random UUID, never the anonymousId — the old + // telemetry id must not survive a wipe in any form. + expect(config.bucketSeed).not.toBe(config.anonymousId); expect(JSON.stringify(stateFile())).not.toContain(config.anonymousId); }); @@ -221,11 +236,15 @@ describe("install-state rollover (breaker survives a config re-mint)", () => { expect(stateFile()["markerAt"]).toBe(minted); }); - it("a corrupted state file reads as absent rather than breaking the mint", () => { + it("a corrupted state file never breaks the mint, and is not counted as fresh", () => { fsState.files.set(STATE_PATH, "{not valid json"); const config = readConfig(); - expect(config.predecessorFound).toBe(false); expect(config.anonymousId).toBeTruthy(); + // Previously asserted `false` here. That was the bug: a machine whose + // record we lost is not a new machine, and reporting it as one understated + // recoverable churn — the only thing predecessorFound measures. + expect(config.predecessorFound).toBe(true); + expect(config.stateFileCorrupt).toBe(true); // And the corrupt file was replaced with a valid marker by the mint's write. expect(stateFile()["markerAt"]).toEqual(expect.any(String)); }); @@ -292,3 +311,370 @@ describe("install-state rollover (breaker survives a config re-mint)", () => { expect(fsState.files.has(LEGACY_STATE_PATH)).toBe(false); }); }); + +describe("bucket-seed carryover (cohorts survive a config wipe)", () => { + let readConfig: typeof import("./config.js").readConfig; + let readConfigFresh: typeof import("./config.js").readConfigFresh; + let writeConfig: typeof import("./config.js").writeConfig; + let CONFIG_PATH: typeof import("./config.js").CONFIG_PATH; + let STATE_PATH: typeof import("./config.js").STATE_PATH; + + beforeEach(async () => { + fsState.files.clear(); + vi.resetModules(); + ({ readConfig, readConfigFresh, writeConfig, CONFIG_PATH, STATE_PATH } = + await import("./config.js")); + }); + + it("a fresh install mints a seed distinct from its anonymousId and mirrors it to the state file", () => { + const config = readConfig(); + expect(config.bucketSeed).toBeTruthy(); + expect(config.bucketSeed).not.toBe(config.anonymousId); + const state = JSON.parse(fsState.files.get(STATE_PATH) as string) as { bucketSeed?: string }; + expect(state.bucketSeed).toBe(config.bucketSeed); + }); + + it("the seed survives a config.json re-mint — cohorts hold, only the id re-rolls", () => { + const first = readConfig(); + fsState.files.delete(CONFIG_PATH); + const second = readConfigFresh(); + expect(second.bucketSeed).toBe(first.bucketSeed); + expect(second.anonymousId).not.toBe(first.anonymousId); + }); + + // The counterpart, and the reason install-state shares CONFIG_DIR: a seed + // that outlived `rm -rf ~/.hyperframes` would be a persistent identifier + // defeating the only reset the user has. + it("the seed does NOT survive deleting the config dir — that reset is real", () => { + const first = readConfig(); + fsState.files.delete(CONFIG_PATH); + fsState.files.delete(STATE_PATH); + const reborn = readConfigFresh(); + expect(reborn.bucketSeed).toBeTruthy(); + expect(reborn.bucketSeed).not.toBe(first.bucketSeed); + expect(reborn.predecessorFound).toBe(false); + }); + + it("adopts a pre-move seed so the migration does not reshuffle a live cohort", () => { + fsState.files.set( + LEGACY_STATE_PATH, + JSON.stringify({ markerAt: "2026-07-28T00:00:00.000Z", bucketSeed: "seed-from-old-path" }), + ); + expect(readConfig().bucketSeed).toBe("seed-from-old-path"); + expect(fsState.files.has(LEGACY_STATE_PATH), "legacy copy removed").toBe(false); + }); + + it("the seed survives config corruption via the same mint path", () => { + const first = readConfig(); + fsState.files.set(CONFIG_PATH, "{not valid json"); + expect(readConfigFresh().bucketSeed).toBe(first.bucketSeed); + }); + + it("the state-file seed is write-once — a later install never overwrites the lineage seed", () => { + const first = readConfig(); + fsState.files.delete(CONFIG_PATH); + const second = readConfigFresh(); + // Force more config writes from the second install; the state seed must not move. + second.commandCount = 7; + writeConfig(second); + const state = JSON.parse(fsState.files.get(STATE_PATH) as string) as { bucketSeed?: string }; + expect(state.bucketSeed).toBe(first.bucketSeed); + }); + + it("a legacy config without a seed is backfilled ONCE and stays stable across fresh reads", () => { + const base = readConfig(); + const { bucketSeed: _dropped, ...legacy } = base; + fsState.files.set(CONFIG_PATH, JSON.stringify(legacy)); + fsState.files.delete(STATE_PATH); // no lineage either — pure legacy machine + const first = readConfigFresh(); + expect(first.bucketSeed).toBeTruthy(); + const second = readConfigFresh(); + expect(second.bucketSeed).toBe(first.bucketSeed); // persisted, not re-rolled per read + }); + + it("a legacy config adopts the lineage seed from the state file when one exists", () => { + const base = readConfig(); // wrote the state file with a seed + const lineageSeed = base.bucketSeed; + const { bucketSeed: _dropped, ...legacy } = base; + fsState.files.set(CONFIG_PATH, JSON.stringify(legacy)); + expect(readConfigFresh().bucketSeed).toBe(lineageSeed); + }); +}); + +describe("install-state corruption is distinguishable from absence", () => { + let readConfig: typeof import("./config.js").readConfig; + let readConfigFresh: typeof import("./config.js").readConfigFresh; + let CONFIG_PATH: typeof import("./config.js").CONFIG_PATH; + let STATE_PATH: typeof import("./config.js").STATE_PATH; + + beforeEach(async () => { + fsState.files.clear(); + vi.resetModules(); + ({ readConfig, readConfigFresh, CONFIG_PATH, STATE_PATH } = await import("./config.js")); + }); + + it("reports predecessorFound for an unreadable state file, not a fresh install", () => { + fsState.files.set(STATE_PATH, "{not valid json"); + const config = readConfig(); + // The machine DID have an install; counting it as fresh understates + // recoverable churn, which is the only thing this field measures. + expect(config.predecessorFound).toBe(true); + expect(config.stateFileCorrupt).toBe(true); + }); + + it("leaves stateFileCorrupt absent on a genuinely fresh install", () => { + const config = readConfig(); + expect(config.predecessorFound).toBe(false); + expect(config.stateFileCorrupt).toBeUndefined(); + }); + + it("salvages a bucketSeed when only markerAt is mangled", () => { + // markerAt is regenerable; the seed is not — losing it re-rolls the cohort. + fsState.files.set(STATE_PATH, JSON.stringify({ markerAt: 12345, bucketSeed: "keep-me" })); + fsState.files.delete(CONFIG_PATH); + const config = readConfigFresh(); + expect(config.bucketSeed).toBe("keep-me"); + expect(config.predecessorFound).toBe(true); + }); + + it("treats a parseable file with nothing salvageable as corrupt", () => { + fsState.files.set(STATE_PATH, JSON.stringify({ unrelated: true })); + expect(readConfig().stateFileCorrupt).toBe(true); + }); +}); + +describe("seed backfill write failure is surfaced", () => { + let readConfig: typeof import("./config.js").readConfig; + let CONFIG_PATH: typeof import("./config.js").CONFIG_PATH; + + beforeEach(async () => { + fsState.files.clear(); + vi.resetModules(); + ({ readConfig, CONFIG_PATH } = await import("./config.js")); + }); + + it("warns once when the backfilled seed cannot be persisted", async () => { + policyState.runtimeOverride = null; + // A config predating bucketSeed, on an unwritable home directory. + const seeded = { telemetryEnabled: true, anonymousId: "id", telemetryNoticeShown: true }; + fsState.files.set(CONFIG_PATH, JSON.stringify(seeded)); + const fs = await import("node:fs"); + vi.mocked(fs.writeFileSync).mockImplementation(() => { + throw new Error("EACCES: permission denied"); + }); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + + readConfig(); + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0]?.[0])).toContain("not be stable across runs"); + + warn.mockRestore(); + vi.mocked(fs.writeFileSync).mockImplementation((path, content) => { + fsState.files.set(String(path), String(content)); + }); + }); +}); + +describe("a tripped breaker survives even total marker+seed corruption", () => { + let readConfig: typeof import("./config.js").readConfig; + let STATE_PATH: typeof import("./config.js").STATE_PATH; + + beforeEach(async () => { + fsState.files.clear(); + vi.resetModules(); + ({ readConfig, STATE_PATH } = await import("./config.js")); + }); + + // Dropping the latch re-enrols a machine into an experimental path that + // already FAILED on it — the one outcome install-state exists to prevent. + // So the tripped bit is salvageable independently of the other two fields. + it("keeps deParallelRouterTrialFired when markerAt and bucketSeed are both unusable", () => { + fsState.files.set( + STATE_PATH, + JSON.stringify({ markerAt: 42, bucketSeed: "", deParallelRouterTrialFired: true }), + ); + const config = readConfig(); + expect(config.deParallelRouterTrialFired).toBe(true); + expect(config.predecessorFound).toBe(true); + }); + + it("still reports corrupt when none of the three fields survive", () => { + fsState.files.set(STATE_PATH, JSON.stringify({ markerAt: 42, bucketSeed: "" })); + expect(readConfig().stateFileCorrupt).toBe(true); + }); +}); + +describe("the breaker latch is authoritative from install-state", () => { + let readConfig: typeof import("./config.js").readConfig; + let writeConfigWithResult: typeof import("./config.js").writeConfigWithResult; + let CONFIG_PATH: typeof import("./config.js").CONFIG_PATH; + let STATE_PATH: typeof import("./config.js").STATE_PATH; + + beforeEach(async () => { + fsState.files.clear(); + vi.resetModules(); + ({ readConfig, writeConfigWithResult, CONFIG_PATH, STATE_PATH } = await import("./config.js")); + }); + + // The stale-writer race: a concurrent process rewrites config.json from a + // snapshot taken before the breaker fired, clearing the flag there. Without + // merging the latch on read, the next process re-enrols a machine whose + // router already failed. + it("rehydrates a latched breaker when config.json says otherwise", () => { + fsState.files.set( + STATE_PATH, + JSON.stringify({ markerAt: "2026-07-28T00:00:00.000Z", deParallelRouterTrialFired: true }), + ); + fsState.files.set( + CONFIG_PATH, + JSON.stringify({ telemetryEnabled: true, anonymousId: "id", bucketSeed: "seed" }), + ); + expect(readConfig().deParallelRouterTrialFired).toBe(true); + }); + + it("does not invent a latch when neither store has one", () => { + fsState.files.set(STATE_PATH, JSON.stringify({ markerAt: "2026-07-28T00:00:00.000Z" })); + fsState.files.set( + CONFIG_PATH, + JSON.stringify({ telemetryEnabled: true, anonymousId: "id", bucketSeed: "seed" }), + ); + expect(readConfig().deParallelRouterTrialFired).toBeUndefined(); + }); + + it("reports mirrored:false when the state write fails but config.json lands", async () => { + const config = readConfig(); + const fs = await import("node:fs"); + const { __resetInstallStateSyncForTests } = await import("./config.js"); + __resetInstallStateSyncForTests(); + fsState.files.delete(STATE_PATH); + vi.mocked(fs.writeFileSync).mockImplementation((path, content) => { + if (String(path).startsWith(STATE_PATH)) throw new Error("EACCES"); + fsState.files.set(String(path), String(content)); + }); + + config.deParallelRouterTrialFired = true; + // The config write still succeeds — a failed mirror must never break it — + // but the caller can now see the safety fact reached only one store. + expect(writeConfigWithResult(config)).toEqual({ ok: true, mirrored: false }); + + vi.mocked(fs.writeFileSync).mockImplementation((path, content) => { + fsState.files.set(String(path), String(content)); + }); + }); +}); + +describe("install-state authority — negative latch and seed", () => { + let readConfig: typeof import("./config.js").readConfig; + let readConfigFresh: typeof import("./config.js").readConfigFresh; + let CONFIG_PATH: typeof import("./config.js").CONFIG_PATH; + let STATE_PATH: typeof import("./config.js").STATE_PATH; + + beforeEach(async () => { + fsState.files.clear(); + vi.resetModules(); + ({ readConfig, readConfigFresh, CONFIG_PATH, STATE_PATH } = await import("./config.js")); + }); + + // Only `true` is monotonic across processes. Caching `false` froze a stale + // negative in a long-lived process (the preview server), so a breaker + // tripped by another process was never picked up. + it("re-reads a negative latch: false -> external trip -> stale config -> fresh read is true", () => { + fsState.files.set(STATE_PATH, JSON.stringify({ markerAt: "2026-07-28T00:00:00.000Z" })); + fsState.files.set( + CONFIG_PATH, + JSON.stringify({ telemetryEnabled: true, anonymousId: "id", bucketSeed: "seed" }), + ); + expect(readConfig().deParallelRouterTrialFired).toBeUndefined(); + + // Another process trips the breaker and mirrors it... + fsState.files.set( + STATE_PATH, + JSON.stringify({ markerAt: "2026-07-28T00:00:00.000Z", deParallelRouterTrialFired: true }), + ); + // ...and a stale writer rewrites config.json without the flag. + fsState.files.set( + CONFIG_PATH, + JSON.stringify({ telemetryEnabled: true, anonymousId: "id", bucketSeed: "seed" }), + ); + + expect(readConfigFresh().deParallelRouterTrialFired).toBe(true); + }); + + // The latch got read-side authority; the seed did not, so the two stores + // could hold different seeds indefinitely and a re-mint flipped every cohort. + it("prefers the install-state seed over a restored config.json seed", () => { + fsState.files.set( + STATE_PATH, + JSON.stringify({ markerAt: "2026-07-28T00:00:00.000Z", bucketSeed: "seed-B" }), + ); + fsState.files.set( + CONFIG_PATH, + JSON.stringify({ telemetryEnabled: true, anonymousId: "id", bucketSeed: "seed-A" }), + ); + expect(readConfig().bucketSeed).toBe("seed-B"); + }); + + // A corrupt file OUTSIDE ~/.hyperframes survived the documented reset and + // reported a predecessor forever. + it("deletes an unreadable pre-move state file instead of reporting it forever", () => { + fsState.files.set(LEGACY_STATE_PATH, "{truncated"); + const config = readConfig(); + expect(config.stateFileCorrupt).toBe(true); + expect(fsState.files.has(LEGACY_STATE_PATH), "legacy copy removed").toBe(false); + }); +}); + +describe("an unwritable config dir must not re-roll the seed", () => { + let readConfig: typeof import("./config.js").readConfig; + let readConfigFresh: typeof import("./config.js").readConfigFresh; + + beforeEach(async () => { + fsState.files.clear(); + policyState.runtimeOverride = null; + vi.resetModules(); + ({ readConfig, readConfigFresh } = await import("./config.js")); + }); + + // No config.json AND an unwritable dir: cachedConfig was never set, so the + // next read re-minted and the seed re-rolled — on every command, forever. + it("keeps one seed for the process when the initial write fails", async () => { + const fs = await import("node:fs"); + vi.mocked(fs.writeFileSync).mockImplementation(() => { + throw new Error("EACCES: permission denied"); + }); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + + const first = readConfig(); + const second = readConfig(); + expect(first.bucketSeed).toBeTruthy(); + expect(second.bucketSeed).toBe(first.bucketSeed); + expect(second.anonymousId).toBe(first.anonymousId); + // And the user is told, rather than churning silently. + expect(warn).toHaveBeenCalledTimes(1); + + warn.mockRestore(); + vi.mocked(fs.writeFileSync).mockImplementation((path, content) => { + fsState.files.set(String(path), String(content)); + }); + }); + + it("stays silent for an install that opted out of telemetry", async () => { + policyState.runtimeOverride = "HYPERFRAMES_NO_TELEMETRY"; + const fs = await import("node:fs"); + vi.mocked(fs.writeFileSync).mockImplementation(() => { + throw new Error("EACCES: permission denied"); + }); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + + readConfigFresh(); + // The only consequence is unstable canary cohorts, and an opted-out + // install is never enrolled in one — so this line was pure noise in CI + // logs, on every invocation, unsilenceable. + expect(warn).not.toHaveBeenCalled(); + + warn.mockRestore(); + vi.mocked(fs.writeFileSync).mockImplementation((path, content) => { + fsState.files.set(String(path), String(content)); + }); + }); +}); diff --git a/packages/cli/src/telemetry/config.ts b/packages/cli/src/telemetry/config.ts index 87831bec1d..df587663ef 100644 --- a/packages/cli/src/telemetry/config.ts +++ b/packages/cli/src/telemetry/config.ts @@ -3,6 +3,7 @@ import { join } from "node:path"; import { homedir } from "node:os"; import { randomUUID } from "node:crypto"; import { normalizeErrorMessage } from "../utils/errorMessage.js"; +import { telemetryRuntimeOverride } from "./policy.js"; // --------------------------------------------------------------------------- // Config directory: ~/.hyperframes/ @@ -29,8 +30,8 @@ const CONFIG_FILE = join(CONFIG_DIR, "config.json"); // that churn: no shared schema to migrate on upgrade, and one write at first // mint instead of one per command. // -// It carries exactly two facts, and no identity — no anonymousId, no -// counters, nothing linking the old install to the new one: +// It carries exactly three facts, and no telemetry identity — no anonymousId, +// no counters, nothing emitted that links the old install to the new one: // // 1. `markerAt` — "a hyperframes install existed on this machine". Now that // it shares CONFIG_DIR, `predecessorFound` measures the churn we care @@ -39,6 +40,10 @@ const CONFIG_FILE = join(CONFIG_DIR, "config.json"); // 2. `deParallelRouterTrialFired` — the DE parallel-router circuit // breaker's tripped state, so a config re-mint does not re-enrol an // install into an experimental path that already FAILED on this machine. +// 3. `bucketSeed` — the canary bucketing unit, so a re-mint does not +// reshuffle cohorts mid-rollout. Never transmitted; only the resulting +// true/false assignments are. Sharing CONFIG_DIR is what keeps it from +// being a persistent identifier that outlives the user's reset. // // Removal path: delete ~/.hyperframes (or just this file). `hyperframes // telemetry status` prints its exact location. @@ -56,23 +61,141 @@ interface InstallState { markerAt: string; /** Rolled-over circuit-breaker state — see HyperframesConfig's field. */ deParallelRouterTrialFired?: boolean; + /** + * The canary bucketing seed — see HyperframesConfig's field. Write-once + * within this config dir: the first install's seed wins forever, so a + * config.json re-mint re-rolls the telemetry id but NOT the canary cohorts. + * Deleting ~/.hyperframes takes the seed with it, by design. + */ + bucketSeed?: string; +} + +/** + * Why there is no usable state: the file isn't there, or it is but couldn't be + * read/parsed. Distinguished because they mean opposite things for churn + * measurement — "absent" is a genuinely new machine, "corrupt" is a machine we + * already knew whose record we lost. Collapsing them into one `null` reported + * every corruption as a fresh install and biased `predecessorFound` low. + */ +type InstallStateMiss = "absent" | "corrupt"; + +/** + * Parse one state file. + * + * A malformed `markerAt` no longer discards the record. `markerAt` is + * regenerable — it is only a timestamp — while `bucketSeed` is not: losing it + * silently re-rolls the install's canary cohort. So a partial corruption that + * leaves a usable seed keeps the seed and restamps the marker. + */ +function salvageInstallState(parsed: Partial): InstallState | InstallStateMiss { + const markerAt = typeof parsed.markerAt === "string" ? parsed.markerAt : undefined; + const bucketSeed = parseNonEmptyString(parsed.bucketSeed); + const fired = parsed.deParallelRouterTrialFired === true ? true : undefined; + // Each of the three is independently salvageable, and the tripped breaker + // most of all: dropping it re-enrols a machine into an experimental path + // that already FAILED there, which is the one outcome this file exists to + // prevent. `markerAt` is only a timestamp and can be restamped. + if (markerAt === undefined && bucketSeed === undefined && fired === undefined) return "corrupt"; + return { + markerAt: markerAt ?? new Date().toISOString(), + deParallelRouterTrialFired: fired, + bucketSeed, + }; } -/** Parse one state file; any parse/shape failure reads as absent. */ -function parseInstallState(file: string): InstallState | null { +function parseInstallState(file: string): InstallState | InstallStateMiss { + if (!existsSync(file)) return "absent"; try { - if (!existsSync(file)) return null; - const parsed = JSON.parse(readFileSync(file, "utf-8")) as Partial; - if (typeof parsed.markerAt !== "string") return null; - return { - markerAt: parsed.markerAt, - deParallelRouterTrialFired: parsed.deParallelRouterTrialFired === true ? true : undefined, - }; + return salvageInstallState(JSON.parse(readFileSync(file, "utf-8")) as Partial); } catch { - return null; + return "corrupt"; } } +// Once per process: the backfill runs on every readConfig until it lands, and +// a read-only home directory would otherwise print on every single command. +let seedBackfillWarned = false; + +/** + * The seed backfill could not be persisted, so this install will mint a + * different seed next invocation and silently churn its canary cohort. Rare + * (unwritable ~/.hyperframes) but invisible without this, and it makes the + * "backfilled once" contract in the field's docstring false. + */ +function warnSeedBackfillFailed(error: string | undefined): void { + if (seedBackfillWarned) return; + seedBackfillWarned = true; + // Suppressed when the user has opted out of telemetry: the only consequence + // of the failed write is unstable CANARY cohorts, and an opted-out install + // is not enrolled in any. Printing anyway put an unsilenceable line into + // CI render logs on every invocation for a user who wants no telemetry at + // all. `hyperframes doctor` still surfaces it on demand. + if (telemetryRuntimeOverride() !== null) return; + console.warn( + `[hyperframes] Could not persist telemetry config${error ? `: ${error}` : ""}. ` + + "Canary cohort assignment will not be stable across runs.", + ); +} + +/** + * One-time backfill for configs predating the bucket seed: prefer the seed a + * previous install recorded, else mint. Mutates `config` in place. + * + * Persisting is the whole point — an unpersisted seed re-rolls next process, + * silently churning this install's cohort on every invocation. `~/.hyperframes` + * being unwritable (root-owned after a sudo mishap, read-only mount, disk full) + * is exactly when that happens, so it says so once rather than failing + * invisibly. + */ +function backfillBucketSeed(config: HyperframesConfig): void { + const recorded = readInstallState(); + config.bucketSeed = (isInstallState(recorded) ? recorded.bucketSeed : undefined) ?? randomUUID(); + const write = writeConfigWithResult(config); + if (!write.ok) warnSeedBackfillFailed(write.error); +} + +// ONLY the positive is cached. The latch is monotonic across processes in one +// direction: once some process trips it, it stays tripped. A cached `false` +// is not monotonic — another process can trip the breaker while this one is +// alive, and a long-lived process (the preview/studio server) would then hold +// a stale negative for hours. So `true` short-circuits and `false` re-reads. +let latchedFiredSeen = false; + +/** + * Is the breaker latched according to install-state? + * + * Merged into EVERY effective read, which is what makes install-state + * authoritative for the latch rather than merely a mirror of config.json. + * Without this a failed mirror, or a stale concurrent writer that rewrites + * config.json from a pre-trip snapshot, leaves state latched and config + * false — and the next process happily re-enrols a machine whose router + * already failed. Verifying the write is not enough on its own; the read has + * to prefer the safety fact. + */ +function installStateLatchedFired(): boolean { + if (latchedFiredSeen) return true; + const state = readInstallState(); + latchedFiredSeen = isInstallState(state) && state.deParallelRouterTrialFired === true; + return latchedFiredSeen; +} + +// Same shape as the latch memo, and same reason for existing: readConfig is +// hot. A seed never changes once recorded, so the positive is cacheable. +let seedFromStateFile: string | undefined; + +/** The seed install-state has recorded for this machine, if any. */ +function installStateSeed(): string | undefined { + if (seedFromStateFile !== undefined) return seedFromStateFile; + const state = readInstallState(); + if (isInstallState(state) && state.bucketSeed !== undefined) seedFromStateFile = state.bucketSeed; + return seedFromStateFile; +} + +/** Narrow the parse result to a usable record. */ +function isInstallState(value: InstallState | InstallStateMiss): value is InstallState { + return typeof value !== "string"; +} + /** * Read the install-state file, adopting the pre-move copy if this machine * still has one. @@ -82,14 +205,23 @@ function parseInstallState(file: string): InstallState | null { * cleared), and failing to delete the legacy copy is not an error — it is * re-read harmlessly next time. */ -function readInstallState(): InstallState | null { +function readInstallState(): InstallState | InstallStateMiss { const current = parseInstallState(STATE_FILE); - if (current !== null) { + if (isInstallState(current)) { removeLegacyStateFile(); return current; } const legacy = parseInstallState(LEGACY_STATE_FILE); - if (legacy === null) return null; + if (!isInstallState(legacy)) { + // Unreadable legacy copy: nothing to migrate, so drop it here too. It + // used to survive, and since it lives OUTSIDE ~/.hyperframes it then + // reported predecessorFound/stateFileCorrupt forever on a machine the + // user had already reset with `rm -rf ~/.hyperframes` — poisoning the one + // metric this file exists to produce. + if (legacy === "corrupt") removeLegacyStateFile(); + // Corruption at EITHER location still means this machine had an install. + return current === "corrupt" || legacy === "corrupt" ? "corrupt" : "absent"; + } try { writeInstallState(legacy); removeLegacyStateFile(); @@ -136,48 +268,101 @@ function writeInstallState(next: InstallState): void { * no breaker write site can forget it. Never throws — same contract as the * rest of this file, telemetry must not break the CLI. */ +function sameInstallState(a: InstallState, b: InstallState): boolean { + return ( + a.deParallelRouterTrialFired === b.deParallelRouterTrialFired && a.bucketSeed === b.bucketSeed + ); +} + +/** Latching: once tripped (by any install in this config dir), stays tripped. */ +function latchedFired(state: InstallState | null, config: HyperframesConfig): true | undefined { + return ( + state?.deParallelRouterTrialFired === true || + config.deParallelRouterTrialFired === true || + undefined + ); +} + /** What the state file should say after this config write; null = already correct. */ -function nextInstallState(state: InstallState | null, wantFired: boolean): InstallState | null { - const hadFired = state?.deParallelRouterTrialFired === true; - if (state !== null && (hadFired || !wantFired)) return null; - // Every path reaching here has hadFired === false (state is either null, or - // the guard above already returned when hadFired was true) — the field is - // simply wantFired, not a merge of the two (review nit, two independent - // reviewers). - return { +function nextInstallState( + state: InstallState | null, + config: HyperframesConfig, +): InstallState | null { + const next: InstallState = { markerAt: state?.markerAt ?? new Date().toISOString(), - deParallelRouterTrialFired: wantFired || undefined, + deParallelRouterTrialFired: latchedFired(state, config), + // Write-once: an existing seed always wins, so cohorts stay + // anchored to the FIRST install in this config dir, not the latest one. + bucketSeed: state?.bucketSeed ?? config.bucketSeed, }; + if (state !== null && sameInstallState(state, next)) return null; + return next; +} + +/** @returns false if the mirror could not be written this call. */ +/** The write half, split out to keep the memo bookkeeping legible. */ +function applyInstallState(config: HyperframesConfig, wantFired: boolean): void { + const read = readInstallState(); + const state = isInstallState(read) ? read : null; + const next = nextInstallState(state, config); + if (next !== null) writeInstallState(next); + stateMarkerSynced = true; + stateFiredSynced = wantFired || state?.deParallelRouterTrialFired === true; + if (wantFired) latchedFiredSeen = true; + if (next?.bucketSeed !== undefined) seedFromStateFile = next.bucketSeed; } -function syncInstallState(config: HyperframesConfig): void { +function syncInstallState(config: HyperframesConfig): boolean { const wantFired = config.deParallelRouterTrialFired === true; - if (stateMarkerSynced && (stateFiredSynced || !wantFired)) return; + if (stateMarkerSynced && (stateFiredSynced || !wantFired)) return true; try { - const state = readInstallState(); - const next = nextInstallState(state, wantFired); - if (next !== null) writeInstallState(next); - stateMarkerSynced = true; - stateFiredSynced = wantFired || state?.deParallelRouterTrialFired === true; + applyInstallState(config, wantFired); + return true; } catch { // Leave the memo unset so a later write retries. + return false; } } +/** + * Mint a fresh config, persist it, and cache it EVEN IF the write failed. + * + * Without the unconditional cache the next readConfig in the same process + * re-minted, re-rolling bucketSeed along with the id — and across processes an + * unwritable ~/.hyperframes (read-only mount, root-owned after a sudo run, + * full disk) meant a fresh cohort on every single command, which is exactly + * the unbounded cumulative exposure the seed exists to prevent. + */ +function mintAndCacheConfig(): HyperframesConfig { + const config = mintConfig(); + const write = writeConfigWithResult(config); + if (!write.ok) warnSeedBackfillFailed(write.error); + cachedConfig = { ...config }; + return { ...config }; +} + /** * Build a brand-new config for an install with no (readable) config file, * consulting the install-state file for what a previous install on this * machine left behind. */ function mintConfig(): HyperframesConfig { - const state = readInstallState(); + const read = readInstallState(); + const state = isInstallState(read) ? read : null; return { ...DEFAULT_CONFIG, anonymousId: randomUUID(), - predecessorFound: state !== null, + // A corrupt record still means this machine had an install — reporting it + // as `false` would count a partial disk write as a brand-new machine and + // understate recoverable churn, which is the one thing this field exists + // to measure. `undefined` on configs minted before the field existed. + predecessorFound: state !== null || read === "corrupt", + stateFileCorrupt: read === "corrupt" ? true : undefined, // The rollover itself: a breaker tripped by a previous install on this - // machine stays tripped for the new one. + // machine stays tripped for the new one, and the canary bucketing seed is + // inherited so cohorts hold across a config.json re-mint. deParallelRouterTrialFired: state?.deParallelRouterTrialFired === true ? true : undefined, + bucketSeed: state?.bucketSeed ?? randomUUID(), }; } @@ -259,11 +444,38 @@ export interface HyperframesConfig { /** * Whether a previous install's state marker existed on this machine when * this config was minted. Attached to telemetry so the fraction of fresh - * installs that are RECOVERABLE churn (config wiped, machine persisted) is + * installs that are RECOVERABLE churn (config.json re-minted, install-state + * survived) is * measurable directly. `undefined` on configs minted before this field * existed — a different fact from `false` (minted fresh, no predecessor). */ predecessorFound?: boolean; + /** + * The install-state file existed but could not be read. Separates "we lost + * this machine's record" from "genuinely new machine" in the churn split — + * without it a partial disk write is indistinguishable from a fresh install. + * Absent (not false) in the normal case, so it costs nothing on the wire. + */ + stateFileCorrupt?: boolean; + /** + * The unit canary percentages bucket on — deliberately NOT the anonymousId. + * A fresh random UUID, mirrored write-once into the install-state file and + * inherited at mint, so a config.json RE-MINT re-rolls the telemetry id but + * keeps this install's canary cohorts: no cumulative-exposure drift from + * corruption, and before/after comparisons survive one. + * + * A re-mint is the churn that actually happens — config.json is rewritten on + * every command and every render, and readConfig recovers from any + * parse/permission/IO failure by minting fresh. Deleting ~/.hyperframes + * deliberately does NOT survive: install-state lives in that same directory + * precisely so the user's reset is a real reset. A seed that outlived it + * would be a persistent identifier defeating the only lever they have. + * + * Never emitted in telemetry (only the resulting true/false assignments + * are), so it does not link the old id to the new one server-side. + * Backfilled once for configs predating the field. + */ + bucketSeed?: string; /** * Ring of the last few local renders (newest last). `hyperframes feedback` * attaches these ids — which are the `render_job_id` / @@ -310,65 +522,119 @@ const DEFAULT_CONFIG: HyperframesConfig = { let cachedConfig: HyperframesConfig | null = null; +/** A non-empty string, or undefined — hand-edited configs can carry anything. */ +function parseNonEmptyString(value: unknown): string | undefined { + return typeof value === "string" && value.length > 0 ? value : undefined; +} + +/** Shape-validate the recent-renders ring from a parsed config. */ +function parseRecentRenders(value: unknown): RecentRenderRecord[] | undefined { + if (!Array.isArray(value)) return undefined; + return value + .filter( + (r): r is RecentRenderRecord => + typeof r === "object" && + r !== null && + typeof (r as RecentRenderRecord).id === "string" && + typeof (r as RecentRenderRecord).at === "string" && + typeof (r as RecentRenderRecord).ok === "boolean", + ) + .slice(-MAX_RECENT_RENDERS); +} + /** * Read the config file, creating it with defaults if it doesn't exist. * Returns a mutable copy — call `writeConfig()` to persist changes. */ +/** + * Materialize a parsed config object, applying defaults and the explicit + * type guards. Split out of readConfig purely for size — that function is + * otherwise one long object literal plus four control-flow branches. + */ +/** Fields that are pure passthrough — no default, no validation. */ +function passthroughFields(parsed: Partial): Partial { + return { + lastUpdateCheck: parsed.lastUpdateCheck, + latestVersion: parsed.latestVersion, + lastStalePinNoticeAt: parsed.lastStalePinNoticeAt, + pendingUpdate: parsed.pendingUpdate, + completedUpdate: parsed.completedUpdate, + lastSkillsCheck: parsed.lastSkillsCheck, + skillsUpdateAvailable: parsed.skillsUpdateAvailable, + skillsOutdatedCount: parsed.skillsOutdatedCount, + skillsMissingCount: parsed.skillsMissingCount, + skillsRemovedCount: parsed.skillsRemovedCount, + }; +} + +/** + * Fields that need an explicit type guard or a cross-store merge, split from + * the plain defaults so neither block is complex on its own. + */ +function guardedFields(parsed: Partial): Partial { + return { + // Explicit `=== true`/typeof-number checks rather than a truthy/nullish + // read — a hand-edited or corrupted config could plausibly carry a + // non-boolean/non-number JSON value (e.g. the STRING "false", which is + // truthy in JS) for these two fields specifically, since they're read + // with a bare truthy check at the call site (review finding). + // `|| installStateLatchedFired()` — a latch recorded in install-state + // wins over an untripped config.json, never the reverse. + deParallelRouterTrialFired: + parsed.deParallelRouterTrialFired === true || installStateLatchedFired() ? true : undefined, + deParallelRouterTrialRenderCount: + typeof parsed.deParallelRouterTrialRenderCount === "number" + ? parsed.deParallelRouterTrialRenderCount + : undefined, + predecessorFound: + typeof parsed.predecessorFound === "boolean" ? parsed.predecessorFound : undefined, + stateFileCorrupt: parsed.stateFileCorrupt === true ? true : undefined, + // Install-state wins, exactly as it does for the latch. Without this the + // two stores could hold different seeds indefinitely: nextInstallState + // is write-once (state's seed always survives), but the read took + // config.json's blindly — so restoring or syncing only config.json left + // the install bucketing on A while install-state kept B, and the next + // re-mint silently flipped every cohort at once. + bucketSeed: installStateSeed() ?? parseNonEmptyString(parsed.bucketSeed), + }; +} + +function materializeConfig(parsed: Partial): HyperframesConfig { + return { + ...passthroughFields(parsed), + telemetryEnabled: parsed.telemetryEnabled ?? DEFAULT_CONFIG.telemetryEnabled, + anonymousId: parsed.anonymousId || randomUUID(), + telemetryNoticeShown: parsed.telemetryNoticeShown ?? DEFAULT_CONFIG.telemetryNoticeShown, + commandCount: parsed.commandCount ?? DEFAULT_CONFIG.commandCount, + renderSuccessCount: parsed.renderSuccessCount ?? DEFAULT_CONFIG.renderSuccessCount, + lastFeedbackPromptAt: parsed.lastFeedbackPromptAt ?? DEFAULT_CONFIG.lastFeedbackPromptAt, + ...guardedFields(parsed), + recentRenders: parseRecentRenders(parsed.recentRenders), + }; +} + export function readConfig(): HyperframesConfig { if (cachedConfig) return { ...cachedConfig }; - if (!existsSync(CONFIG_FILE)) { - const config = mintConfig(); - writeConfig(config); - return config; - } + if (!existsSync(CONFIG_FILE)) return mintAndCacheConfig(); try { const raw = readFileSync(CONFIG_FILE, "utf-8"); const parsed = JSON.parse(raw) as Partial; - const config: HyperframesConfig = { - telemetryEnabled: parsed.telemetryEnabled ?? DEFAULT_CONFIG.telemetryEnabled, - anonymousId: parsed.anonymousId || randomUUID(), - telemetryNoticeShown: parsed.telemetryNoticeShown ?? DEFAULT_CONFIG.telemetryNoticeShown, - commandCount: parsed.commandCount ?? DEFAULT_CONFIG.commandCount, - renderSuccessCount: parsed.renderSuccessCount ?? DEFAULT_CONFIG.renderSuccessCount, - lastFeedbackPromptAt: parsed.lastFeedbackPromptAt ?? DEFAULT_CONFIG.lastFeedbackPromptAt, - lastUpdateCheck: parsed.lastUpdateCheck, - latestVersion: parsed.latestVersion, - lastStalePinNoticeAt: parsed.lastStalePinNoticeAt, - pendingUpdate: parsed.pendingUpdate, - completedUpdate: parsed.completedUpdate, - lastSkillsCheck: parsed.lastSkillsCheck, - skillsUpdateAvailable: parsed.skillsUpdateAvailable, - skillsOutdatedCount: parsed.skillsOutdatedCount, - skillsMissingCount: parsed.skillsMissingCount, - skillsRemovedCount: parsed.skillsRemovedCount, - // Explicit `=== true`/typeof-number checks rather than a truthy/nullish - // read — a hand-edited or corrupted config could plausibly carry a - // non-boolean/non-number JSON value (e.g. the STRING "false", which is - // truthy in JS) for these two fields specifically, since they're read - // with a bare truthy check at the call site (review finding). - deParallelRouterTrialFired: parsed.deParallelRouterTrialFired === true ? true : undefined, - deParallelRouterTrialRenderCount: - typeof parsed.deParallelRouterTrialRenderCount === "number" - ? parsed.deParallelRouterTrialRenderCount - : undefined, - predecessorFound: - typeof parsed.predecessorFound === "boolean" ? parsed.predecessorFound : undefined, - recentRenders: Array.isArray(parsed.recentRenders) - ? parsed.recentRenders - .filter( - (r): r is RecentRenderRecord => - typeof r === "object" && - r !== null && - typeof (r as RecentRenderRecord).id === "string" && - typeof (r as RecentRenderRecord).at === "string" && - typeof (r as RecentRenderRecord).ok === "boolean", - ) - .slice(-MAX_RECENT_RENDERS) - : undefined, - }; + const config = materializeConfig(parsed); + + // One-time backfill for configs predating the bucket seed: prefer the + // recorded seed if a previous install already wrote one, else mint. + // Persisted immediately — an unpersisted seed would re-roll every process. + if (config.bucketSeed === undefined) { + backfillBucketSeed(config); + // Cache even if the write failed, so the seed is at least stable for + // the life of this process (a re-roll per readConfigFresh would flip + // cohorts mid-session). + cachedConfig = config; + return { ...config }; + } cachedConfig = config; return { ...config }; @@ -417,7 +683,11 @@ export function writeConfig(config: HyperframesConfig): boolean { return writeConfigWithResult(config).ok; } -export type ConfigWriteResult = { ok: true } | { ok: false; error: string }; +export type ConfigWriteResult = + // `mirrored: false` means config.json landed but the install-state mirror + // did not. Not a write failure — the latch is merged back in on read — but + // callers persisting a tripped breaker may want to retry or warn. + { ok: true; mirrored?: false } | { ok: false; error: string }; /** * Persist config and retain the failure reason for user-facing commands that @@ -431,9 +701,13 @@ export function writeConfigWithResult(config: HyperframesConfig): ConfigWriteRes renameSync(tmpFile, CONFIG_FILE); cachedConfig = { ...config }; // Mirror into the install-state file (marker + tripped breaker) so no - // breaker write site has to remember to do it. - syncInstallState(config); - return { ok: true }; + // breaker write site has to remember to do it. A failed mirror does NOT + // fail the config write — config.json landed, and the latch is merged + // back in on read — but it is reported rather than swallowed so a caller + // persisting a tripped breaker can tell the safety fact only reached one + // of the two stores. + const mirrored = syncInstallState(config); + return mirrored ? { ok: true } : { ok: true, mirrored: false }; } catch (error) { // Non-fatal — telemetry should never break the CLI return { ok: false, error: normalizeErrorMessage(error) }; diff --git a/packages/cli/src/telemetry/policy.test.ts b/packages/cli/src/telemetry/policy.test.ts index 4894f29fcf..4ffc93f290 100644 --- a/packages/cli/src/telemetry/policy.test.ts +++ b/packages/cli/src/telemetry/policy.test.ts @@ -5,7 +5,9 @@ async function loadPolicy(options?: { devMode?: boolean; apiKey?: string }) { vi.doMock("../utils/env.js", () => ({ isDevMode: () => options?.devMode ?? false, })); - vi.doMock("./transport.js", () => ({ + // The key moved to a leaf module to break a config -> policy -> transport + // -> config import cycle; policy.ts reads it from there now. + vi.doMock("./posthogKey.js", () => ({ POSTHOG_API_KEY: options?.apiKey ?? "phc_test", })); return import("./policy.js"); diff --git a/packages/cli/src/telemetry/policy.ts b/packages/cli/src/telemetry/policy.ts index 78e97df84a..8daa7cfefb 100644 --- a/packages/cli/src/telemetry/policy.ts +++ b/packages/cli/src/telemetry/policy.ts @@ -1,5 +1,5 @@ import { isDevMode } from "../utils/env.js"; -import { POSTHOG_API_KEY } from "./transport.js"; +import { POSTHOG_API_KEY } from "./posthogKey.js"; export type TelemetryStatusSource = | "config" diff --git a/packages/cli/src/telemetry/posthogKey.ts b/packages/cli/src/telemetry/posthogKey.ts new file mode 100644 index 0000000000..fe72aa24ae --- /dev/null +++ b/packages/cli/src/telemetry/posthogKey.ts @@ -0,0 +1,10 @@ +/** + * The PostHog write-only ingest key, as a LEAF module. + * + * It lives here rather than in transport.ts because `policy.ts` needs it (an + * unconfigured key is itself a telemetry opt-out) and importing transport for + * one constant created `config.ts -> policy.ts -> transport.ts -> config.ts`. + * A cycle through the telemetry config is the kind that bites at module-init + * time, so the constant moved instead of the dependency being tolerated. + */ +export const POSTHOG_API_KEY = "phc_zjjbX0PnWxERXrMHhkEJWj9A9BhGVLRReICgsfTMmpx"; diff --git a/packages/cli/src/telemetry/transport.ts b/packages/cli/src/telemetry/transport.ts index 9ec9ac0031..42fe4971cd 100644 --- a/packages/cli/src/telemetry/transport.ts +++ b/packages/cli/src/telemetry/transport.ts @@ -1,10 +1,11 @@ import { spawn } from "node:child_process"; import { randomUUID } from "node:crypto"; +import { POSTHOG_API_KEY } from "./posthogKey.js"; import { readConfig } from "./config.js"; // This is a public project API key — safe to embed in client-side code. // It only allows writing events, not reading data. -export const POSTHOG_API_KEY = "phc_zjjbX0PnWxERXrMHhkEJWj9A9BhGVLRReICgsfTMmpx"; + const POSTHOG_HOST = "https://us.i.posthog.com"; const FLUSH_TIMEOUT_MS = 5_000; diff --git a/packages/core/package-subpaths.json b/packages/core/package-subpaths.json index 62bc92fe03..2a61955dc0 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 a4454202a9..f8d5361406 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/core/src/canary.test.ts b/packages/core/src/canary.test.ts new file mode 100644 index 0000000000..03da853ca2 --- /dev/null +++ b/packages/core/src/canary.test.ts @@ -0,0 +1,370 @@ +import { describe, expect, it } from "vitest"; +import { canaryBucket, evaluateCanary, parseCanaryOverride, type CanaryInput } from "./canary.js"; +import { CANARIES, canaryEnvVar, findCanary, overdueCanaries } from "./canaryRegistry.js"; +import { CANARY_FEATURE_PREFIX, canaryFeatureKey, canaryFeatureProperties } from "./canary.js"; + +const base = (over: Partial = {}): CanaryInput => ({ + feature: "test-feature", + unitId: "db0c1f4a-b95e-4c35-90c6-1a15bd76f717", + percentage: 10, + ...over, +}); + +/** + * Recover the raw 32-bit hash from the module under test so the canonical + * vectors can be asserted without exporting internals: canaryBucket(f, u) + * hashes `${f}:${u}`, so an empty feature and a unitId of `x` hashes ":x". + * Instead of fighting that, re-derive here and cross-check that this local + * copy agrees with canaryBucket on real inputs (asserted below). + */ +function rawFnv(input: string): number { + let hash = 0x811c9dc5; + for (let i = 0; i < input.length; i++) { + hash ^= input.charCodeAt(i); + hash = (hash + ((hash << 1) + (hash << 4) + (hash << 7) + (hash << 8) + (hash << 24))) >>> 0; + } + return hash >>> 0; +} + +/** + * A realistic population: v4-shaped UUIDs, but from a SEEDED PRNG. + * + * These ids feed statistical assertions (share within 1pp, chi-square + * uniformity) whose thresholds are tight enough to fail by chance on a + * genuinely random draw: measured at ~4 failures per 1500 runs for the share + * bound (the pct=50 case has a binomial SD of 0.354pp, so 1pp is only 2.8 + * sigma) and 1 per 1000 for chi-square by its own construction. That made the + * whole @hyperframes/core suite flaky for unrelated PRs. Seeded means the + * population is fixed, so a failure is a real change in the hash — which is + * the only thing these tests are for. + */ +function uuids(n: number, seed = 0x9e3779b9): string[] { + let state = seed >>> 0; + const nextByte = (): number => { + // xorshift32 — deterministic, and uniform enough to stand in for a real + // id population. Not used for anything security-relevant. + state ^= state << 13; + state >>>= 0; + state ^= state >>> 17; + state ^= state << 5; + state >>>= 0; + return state & 0xff; + }; + const hex = (count: number): string => + Array.from({ length: count }, () => nextByte().toString(16).padStart(2, "0")).join(""); + return Array.from( + { length: n }, + () => `${hex(4)}-${hex(2)}-4${hex(2).slice(1)}-a${hex(2).slice(1)}-${hex(6)}`, + ); +} + +describe("fnv1a32 (via canaryBucket)", () => { + it("matches canonical FNV-1a 32-bit vectors", () => { + // canaryBucket hashes `feature:unitId`, so feed the vector as the whole + // string by using an empty feature and reconstructing the separator. + // Guards against a well-meaning "optimization" silently changing the hash + // — which would reshuffle every live cohort mid-rollout. + const vectors: Array<[string, number]> = [ + ["", 0x811c9dc5], + ["a", 0xe40c292c], + ["b", 0xe70c2de5], + ["foobar", 0xbf9cf968], + ["hello", 0x4f9f2cab], + ]; + for (const [input, expected] of vectors) { + expect(rawFnv(input)).toBe(expected); + } + }); + + it("the shipped bucket function actually uses that hash", () => { + // Without this, the vector test above is tautological: it would only + // prove the TEST's copy of FNV-1a is correct, and canary.ts could drift + // to a different hash with every assertion still green. + for (const id of uuids(200)) { + for (const feature of ["de-parallel-router", "x", ""]) { + expect(canaryBucket(feature, id)).toBe(rawFnv(`${feature}:${id}`) % 100); + } + } + }); +}); + +describe("evaluateCanary", () => { + it("is deterministic for the same feature + unit", () => { + const a = evaluateCanary(base()); + const b = evaluateCanary(base()); + expect(a).toEqual(b); + }); + + it("honours an explicit override in both directions, over any percentage", () => { + expect(evaluateCanary(base({ percentage: 0, override: true }))).toEqual({ + enabled: true, + reason: "forced_on", + }); + expect(evaluateCanary(base({ percentage: 100, override: false }))).toEqual({ + enabled: false, + reason: "forced_off", + }); + }); + + it("0% is off for everyone and 100% is on for everyone", () => { + for (const id of uuids(50)) { + expect(evaluateCanary(base({ unitId: id, percentage: 0 })).enabled).toBe(false); + expect(evaluateCanary(base({ unitId: id, percentage: 100 })).enabled).toBe(true); + } + }); + + it("fails closed without a unit id — unknown must never mean a PARTIAL cohort", () => { + for (const id of [undefined, "", " "]) { + expect(evaluateCanary(base({ unitId: id, percentage: 50 }))).toEqual({ + enabled: false, + reason: "no_unit_id", + }); + } + }); + + it("excludes flagged units (CI) from percentage enrolment but not from an override", () => { + expect(evaluateCanary(base({ percentage: 50, exclude: true })).reason).toBe("excluded"); + expect(evaluateCanary(base({ percentage: 50, exclude: true, override: true })).enabled).toBe( + true, + ); + }); + + // 100 is the one percentage where "we don't know who this is" and "this is + // CI" stop mattering: the registry's step 4 says to delete the entry and the + // guard at 100-and-holding, so any population still resolving false here + // would take the new path for the FIRST time at deletion — unstaged, and + // invisible on the dashboard that said it was safe. + it.each([ + ["no unit id", { unitId: undefined }], + ["blank unit id", { unitId: " " }], + ["excluded (CI)", { exclude: true }], + ])("at 100%% enrols %s, so deleting the guard changes nothing", (_label, extra) => { + expect(evaluateCanary(base({ percentage: 100, ...extra }))).toEqual({ + enabled: true, + reason: "in_cohort", + }); + }); + + it("an explicit off still wins at 100%", () => { + expect(evaluateCanary(base({ percentage: 100, override: false })).enabled).toBe(false); + }); + + it("clamps out-of-range and fractional percentages", () => { + expect(evaluateCanary(base({ percentage: -5 })).enabled).toBe(false); + expect(evaluateCanary(base({ percentage: 999 })).enabled).toBe(true); + // 10.9 truncates to 10 — same cohort as an even 10, no surprise widening. + const ids = uuids(300); + const at10 = ids.filter((id) => evaluateCanary(base({ unitId: id, percentage: 10 })).enabled); + const at109 = ids.filter( + (id) => evaluateCanary(base({ unitId: id, percentage: 10.9 })).enabled, + ); + expect(at109).toEqual(at10); + }); +}); + +describe("cohort properties", () => { + it("ramping is INCLUSIVE — widening never drops an already-enrolled install", () => { + // If a ramp reshuffled the cohort, before/after comparisons across the + // ramp would be meaningless and some users would flap in and out. + const ids = uuids(500); + const enrolledAt = (pct: number) => + new Set(ids.filter((id) => evaluateCanary(base({ unitId: id, percentage: pct })).enabled)); + const p5 = enrolledAt(5); + const p25 = enrolledAt(25); + const p100 = enrolledAt(100); + for (const id of p5) expect(p25.has(id)).toBe(true); + for (const id of p25) expect(p100.has(id)).toBe(true); + expect(p25.size).toBeGreaterThan(p5.size); + }); + + it("different features select INDEPENDENT slices of the same population", () => { + // The whole reason the hash includes the feature name: bucketing on the + // unit id alone would hand every simultaneous experiment to one unlucky + // cohort, and make two rollouts impossible to read apart. + const ids = uuids(2000); + const a = new Set( + ids.filter((id) => evaluateCanary({ feature: "feat-a", unitId: id, percentage: 10 }).enabled), + ); + const b = new Set( + ids.filter((id) => evaluateCanary({ feature: "feat-b", unitId: id, percentage: 10 }).enabled), + ); + const overlap = [...a].filter((id) => b.has(id)).length; + // Independent 10% slices overlap ~1% of the population (~20 of 2000). + // Identical slices would overlap ~200. Assert well below that. + expect(overlap).toBeLessThan(70); + expect(a.size).toBeGreaterThan(0); + expect(b.size).toBeGreaterThan(0); + }); + + it("N concurrent canaries enrol installs binomially, not in lockstep", () => { + // The sharpest statement of independence. With 8 canaries at 10% each, + // independent slices give binomial(8, 0.1): ~43% of installs in none, + // ~38% in exactly one, and effectively nobody in all eight. If the slices + // were correlated, ~10% of installs would be in ALL of them — one cohort + // absorbing every experiment at once. + const ids = uuids(20000); + const features = ["a", "b", "c", "d", "e", "f", "g", "h"].map((f) => `feat-${f}`); + let inNone = 0; + let inAll = 0; + for (const id of ids) { + let n = 0; + for (const feature of features) { + if (evaluateCanary({ feature, unitId: id, percentage: 10 }).enabled) n++; + } + if (n === 0) inNone++; + if (n === features.length) inAll++; + } + // binomial: P(0) = 0.9^8 = 43.0% + expect(Math.abs((inNone / ids.length) * 100 - 43.0)).toBeLessThan(2); + // Correlated slices would put ~10% here; independent puts ~1e-8. + expect(inAll).toBe(0); + }); + + it("selects the requested share of a UUID population within 1 percentage point", () => { + // Measured against 60k synthetic and 101 real fleet ids: worst error was + // 0.16pp. A 1pp band is therefore a real guard, not a formality — the + // earlier 0.6x-1.4x band would have passed a badly skewed hash. + const ids = uuids(20000); + for (const pct of [1, 5, 10, 25, 50]) { + const hits = ids.filter( + (id) => evaluateCanary(base({ unitId: id, percentage: pct })).enabled, + ).length; + const actual = (hits / ids.length) * 100; + expect(Math.abs(actual - pct)).toBeLessThan(1); + } + }); + + it("distributes uniformly across all 100 buckets (chi-square)", () => { + // The strongest available guard on the hash: a lumpy hash still yields + // roughly the right TOTAL share while over-loading some buckets, so the + // share test alone can't catch it. + const ids = uuids(30000); + const counts = new Array(100).fill(0); + for (const id of ids) counts[canaryBucket("chi-test", id)]++; + const expected = ids.length / 100; + const chi2 = counts.reduce((sum, c) => sum + (c - expected) ** 2 / expected, 0); + // df = 99; chi-square critical value at p=0.001 is 148.2. + expect(chi2).toBeLessThan(148.2); + expect(Math.min(...counts)).toBeGreaterThan(0); + }); +}); + +describe("parseCanaryOverride", () => { + it("accepts the spellings people actually type", () => { + for (const v of ["1", "true", "TRUE", "on", "yes", " On "]) { + expect(parseCanaryOverride(v)).toBe(true); + } + for (const v of ["0", "false", "FALSE", "off", "no", " Off "]) { + expect(parseCanaryOverride(v)).toBe(false); + } + }); + + it("treats unset, empty and unrecognised values as 'no override'", () => { + // An exported-but-empty var must not force a feature on. + for (const v of [undefined, "", " ", "maybe"]) { + expect(parseCanaryOverride(v)).toBeUndefined(); + } + }); +}); + +describe("registry", () => { + // Also load-bearing for the hash, not just for tidiness: fnv1a32 walks + // charCodeAt, i.e. UTF-16 code units, while reference FNV-1a is byte + // oriented. The two agree only for ASCII. Names are hashed as + // `feature:unit`, so a non-ASCII name (an accented owner tag, an emoji, a + // full-width dash from autocorrect) would silently disagree with every + // other FNV-1a implementation — including any external tool that recomputes + // cohorts. This regex is what makes that unreachable; loosening it means + // fixing the hash first. + it("has unique, ASCII kebab-case names — the hash depends on this", () => { + const names = CANARIES.map((c) => c.name); + expect(new Set(names).size).toBe(names.length); + for (const n of names) { + expect(n).toMatch(/^[a-z0-9]+(-[a-z0-9]+)*$/); + // eslint-disable-next-line no-control-regex -- explicit ASCII range check + expect(n).toMatch(/^[\x00-\x7F]*$/); + } + }); + + // The registry is data, so a ramp is a one-line edit with no code review + // surface. This canary's own description says "ramp only alongside the + // per-install circuit breaker" — without an assertion, bumping it to 5 + // before that wiring lands would go green. + it("keeps de-parallel-router at 0% until the circuit breaker is wired", () => { + expect(findCanary("de-parallel-router")?.percentage).toBe(0); + }); + + it("has in-range percentages and a parseable sunset date", () => { + for (const c of CANARIES) { + expect(c.percentage).toBeGreaterThanOrEqual(0); + expect(c.percentage).toBeLessThanOrEqual(100); + expect(Number.isNaN(Date.parse(`${c.sunsetAfter}T00:00:00Z`))).toBe(false); + expect(c.owner.length).toBeGreaterThan(0); + expect(c.description.length).toBeGreaterThan(0); + } + }); + + it("derives the override env var from the name", () => { + expect(canaryEnvVar("de-parallel-router")).toBe("HF_CANARY_DE_PARALLEL_ROUTER"); + expect(findCanary("de-parallel-router")?.name).toBe("de-parallel-router"); + expect(findCanary("nope")).toBeUndefined(); + }); + + // Deliberately NOT `overdueCanaries()` with the ambient date. That assertion + // reads wall-clock time, so it turns the entire @hyperframes/core suite red + // on a calendar date for every unrelated PR — a broken build nobody caused + // and whose fix is unrelated to the change under test. The registry's own + // freshness is enforced by the pinned dates below plus the sunset REPORT, + // which is advisory rather than a gate. + it("every canary carries a parseable sunset date in the future at authoring time", () => { + const authored = new Date("2026-07-31T00:00:00Z"); + for (const c of CANARIES) { + const sunset = Date.parse(`${c.sunsetAfter}T00:00:00Z`); + expect(Number.isFinite(sunset), `${c.name} has an unparseable sunsetAfter`).toBe(true); + expect(sunset, `${c.name} was authored already-expired`).toBeGreaterThan(authored.getTime()); + } + }); + + it("reports a canary as overdue only AFTER the whole sunset day has passed", () => { + const [first] = CANARIES; + if (!first) return; + const day = first.sunsetAfter; + expect(overdueCanaries(new Date(`${day}T00:00:00Z`))).not.toContain(first.name); + expect(overdueCanaries(new Date(`${day}T23:59:59Z`))).not.toContain(first.name); + const dayAfter = new Date(Date.parse(`${day}T00:00:00Z`) + 86_400_000); + expect(overdueCanaries(dayAfter)).toContain(first.name); + }); +}); + +describe("PostHog flag-shaped properties", () => { + it("namespaces keys so a canary can never alias a real PostHog flag", () => { + // A real flag namespace already exists in this project, owned by the web + // app (e.g. `enable-chat-tab`). Without the `canary-` infix a canary named + // after a real flag would fight it for the same property. + expect(canaryFeatureKey("de-parallel-router")).toBe("$feature/canary-de-parallel-router"); + expect(CANARY_FEATURE_PREFIX.startsWith("$feature/")).toBe(true); + }); + + it("emits every canary, not just enrolled ones", () => { + // Absent vs "false" are different facts: absent = this build predates the + // canary, "false" = this build has it and this install is control. + // Collapsing them makes a ramp unreadable. + const props = canaryFeatureProperties([ + { name: "a", enabled: true }, + { name: "b", enabled: false }, + ]); + expect(props).toEqual({ + "$feature/canary-a": "true", + "$feature/canary-b": "false", + }); + }); + + it("uses string values, matching how PostHog records boolean flags", () => { + const props = canaryFeatureProperties([{ name: "a", enabled: true }]); + expect(typeof props["$feature/canary-a"]).toBe("string"); + }); + + it("is empty when nothing is registered", () => { + expect(canaryFeatureProperties([])).toEqual({}); + }); +}); diff --git a/packages/core/src/canary.ts b/packages/core/src/canary.ts new file mode 100644 index 0000000000..5fb5cd6f31 --- /dev/null +++ b/packages/core/src/canary.ts @@ -0,0 +1,198 @@ +/** + * Canary rollouts — ship a change to a stable slice of installs instead of + * all-or-nothing. + * + * The problem this solves: the repo has ~49 `HF_*` / `PRODUCER_*` boolean + * toggles, and every one of them is binary. A change is either off (and + * therefore untested on real traffic) or on for everyone (and therefore a + * fleet-wide bet). The parallel-drawElement router spent weeks in that gap: + * default-off collected almost no signal, and flipping it default-on exposed + * 100% of eligible installs at once. A percentage slice is the missing rung. + * + * Design notes worth knowing before you add one: + * + * - **Pure and universal.** No fs, no network, no `process` — the caller + * supplies the unit id and the overrides. That keeps this importable from + * the CLI, the producer, the engine, studio-server, the browser-side studio + * bundle, and the embeddable player alike. + * + * - **Independent slices.** The bucket is a hash of `feature:unitId`, NOT of + * `unitId` alone. If every canary bucketed on the id by itself, they would + * all select the SAME installs — one unlucky cohort would receive every + * experiment simultaneously, and no two rollouts could be read + * independently. + * + * - **Ramping is inclusive.** `bucket < percentage` means widening 10 → 25 + * keeps every install that was already at 10. Cohorts never reshuffle, so + * before/after comparisons stay valid across a ramp. + * + * - **Stable per install, for the life of the install.** The same id and + * feature always resolve the same way, with no persisted state to keep in + * sync and nothing to look up at runtime. + */ + +/** Why a canary resolved the way it did. Attach to telemetry — a rollout you + * can't segment by enrolment reason is a rollout you can't debug. */ +export type CanaryReason = + | "forced_on" + | "forced_off" + | "in_cohort" + | "out_of_cohort" + | "no_unit_id" + | "excluded" + // Telemetry is off, so the install is not enrolled. Distinct from + // "excluded" because the caller decides this BEFORE evaluate is reached — + // and because "why is my canary off" has a very different answer in the two + // cases. Never appears in telemetry by construction: an install that + // resolves this way sends nothing. + | "telemetry_opt_out"; + +export interface CanaryDecision { + enabled: boolean; + reason: CanaryReason; + /** 0-99 slot this unit landed in for this feature; undefined when not computed. */ + bucket?: number; +} + +export interface CanaryInput { + /** Registry key, e.g. "de-parallel-router". Part of the hash, so each feature gets its own slice. */ + feature: string; + /** Stable per-install id — the CLI's telemetry `anonymousId`. Missing/blank fails closed. */ + unitId: string | undefined; + /** 0 = off for everyone, 100 = on for everyone. Values outside 0-100 are clamped. */ + percentage: number; + /** + * Explicit override, both directions — support escalations, dogfooding, a + * bisect, or a panic-off. Always wins over the percentage. + */ + override?: boolean | undefined; + /** + * Exclude this unit from percentage-based enrolment (an explicit override + * still applies). Callers pass `isCI` here: CI installs regenerate their + * config constantly, so their ids are ephemeral — they would hop cohorts + * between runs, adding noise to the rollout signal while telling you + * nothing about real users. + */ + exclude?: boolean | undefined; +} + +/** + * FNV-1a (32-bit). Chosen over `node:crypto` deliberately: this module has to + * run in the browser-side studio bundle and the embeddable player too, and a + * six-line hash beats shipping a polyfill or maintaining two code paths. + * Distribution is uniform enough for bucketing (pinned by a test). + * + * ASCII-only by contract. `charCodeAt` yields UTF-16 code units — two + * surrogate halves for an astral character — whereas reference FNV-1a is + * byte-oriented, so the two agree only on ASCII. Both inputs are constrained + * to satisfy that: canary names by the registry's kebab-case assertion in + * `canary.test.ts`, unit ids by being UUIDs. Widening either means switching + * to a UTF-8 encoding here first, which is not free in the browser bundle + * (`TextEncoder` is fine; `Buffer` is not). + */ +function fnv1a32(input: string): number { + let hash = 0x811c9dc5; + for (let i = 0; i < input.length; i++) { + hash ^= input.charCodeAt(i); + // hash * 16777619, kept in 32-bit unsigned range without Math.imul overflow + hash = (hash + ((hash << 1) + (hash << 4) + (hash << 7) + (hash << 8) + (hash << 24))) >>> 0; + } + return hash >>> 0; +} + +/** The 0-99 slot a unit occupies for a given feature. Exported for tests and diagnostics. */ +export function canaryBucket(feature: string, unitId: string): number { + return fnv1a32(`${feature}:${unitId}`) % 100; +} + +/** + * Resolve whether a feature is on for this unit. + * + * Fails closed on a missing id: the canary exists to bound blast radius, so + * "we don't know who this is" must mean "not enrolled", never "enrol + * everyone". + */ +export function evaluateCanary(input: CanaryInput): CanaryDecision { + if (input.override === true) return { enabled: true, reason: "forced_on" }; + if (input.override === false) return { enabled: false, reason: "forced_off" }; + + const pct = Math.max(0, Math.min(100, Math.trunc(input.percentage))); + if (pct <= 0) return { enabled: false, reason: "out_of_cohort" }; + + // Ahead of `exclude` and the unit-id check: at 100 the registry's step 4 + // says to delete the entry and the guard, so anything still resolving false + // here would take the new path for the FIRST time at deletion, unstaged. + // CI and seedless installs are exactly the populations a dashboard cannot + // see, so "100% and holding" looked green while they were never exercised. + if (pct >= 100) return { enabled: true, reason: "in_cohort" }; + + if (input.exclude) return { enabled: false, reason: "excluded" }; + + const unitId = input.unitId?.trim(); + if (!unitId) return { enabled: false, reason: "no_unit_id" }; + + const bucket = canaryBucket(input.feature, unitId); + return bucket < pct + ? { enabled: true, reason: "in_cohort", bucket } + : { enabled: false, reason: "out_of_cohort", bucket }; +} + +/** + * Parse a canary override from an env-var value. + * + * Accepts the spellings people actually type. Returns undefined for + * unset/empty so the percentage decides — matching how the existing HF_* + * knobs treat a set-but-empty var, and avoiding the failure mode where an + * exported-but-empty variable silently forces a feature on. + */ +export function parseCanaryOverride(raw: string | undefined): boolean | undefined { + const v = raw?.trim().toLowerCase(); + if (v === undefined || v === "") return undefined; + if (v === "1" || v === "true" || v === "on" || v === "yes") return true; + if (v === "0" || v === "false" || v === "off" || v === "no") return false; + return undefined; +} + +/** + * Property-name prefix for canary assignments on telemetry events. + * + * PostHog treats `$feature/` as a first-class flag property: breakdowns, + * funnels split by cohort and the experiment surfaces all key on it. Emitting + * assignments in that shape means the analysis tooling works on a canary with + * nothing configured server-side — the decision still happens locally and + * offline, which the render path requires (no render-time network calls, and + * behaviour must not depend on analytics being reachable). + * + * The `canary-` infix is deliberate. A real PostHog flag namespace already + * exists in this project, owned by the web app (e.g. `enable-chat-tab`, set by + * posthog-js). Namespacing guarantees a canary key can never alias a real flag + * key and have the two fight over the same property. + */ +export const CANARY_FEATURE_PREFIX = "$feature/canary-"; + +/** `de-parallel-router` → `$feature/canary-de-parallel-router`. */ +export function canaryFeatureKey(name: string): string { + return `${CANARY_FEATURE_PREFIX}${name}`; +} + +/** + * Build the telemetry properties for a set of resolved canaries. + * + * Emits EVERY registered canary, not just the enrolled ones, because absent + * and `"false"` mean different things: absent is "this build predates the + * canary", `"false"` is "this build has it and this install is not enrolled". + * Collapsing those makes a ramp unreadable — you cannot tell a control group + * from an old version. + * + * Values are the strings `"true"` / `"false"` to match how PostHog records + * boolean flag values, so the property is directly comparable to a real flag. + */ +export function canaryFeatureProperties( + entries: ReadonlyArray<{ name: string; enabled: boolean }>, +): Record { + const props: Record = {}; + for (const entry of entries) { + props[canaryFeatureKey(entry.name)] = entry.enabled ? "true" : "false"; + } + return props; +} diff --git a/packages/core/src/canaryRegistry.ts b/packages/core/src/canaryRegistry.ts new file mode 100644 index 0000000000..f0ad7936c4 --- /dev/null +++ b/packages/core/src/canaryRegistry.ts @@ -0,0 +1,115 @@ +/** + * The canary registry — every staged rollout in the product, in one file. + * + * Why a registry rather than a percentage inlined at each call site: the repo + * already carries 57 loose `HF_*` / `PRODUCER_*` toggles with no index, so + * nobody can answer "what is currently rolling out, to how many people, and + * who owns it" without grepping. One table fixes that, and gives the + * telemetry and `doctor` surfaces something to enumerate. + * + * ## Adding one + * + * 1. Add an entry below. Start at `percentage: 0` and merge that — a canary + * at 0 is dead code you can land safely and ramp without a code review. + * 2. Read it at the decision point via the surface's binding (in the CLI, + * `isCanaryEnabled("your-feature")`). + * 3. Ramp by editing `percentage` in a patch release: 0 → 5 → 25 → 100. + * Widening is inclusive, so the earlier cohort stays enrolled and the + * before/after comparison survives the ramp. + * 4. At 100 and holding, DELETE the entry and the branch it guarded. That is + * the point of `sunsetAfter`. + * + * ## Overriding + * + * `HF_CANARY_` with the feature name upper-snake-cased, e.g. + * `HF_CANARY_DE_PARALLEL_ROUTER=on` (also: off/true/false/1/0/yes/no). + * An override always wins over the percentage, in both directions. + */ + +export interface CanaryDefinition { + /** Registry key. Kebab-case; also the hash input, so renaming reshuffles the cohort. */ + name: string; + /** 0-100. Start at 0, ramp in patch releases. */ + percentage: number; + /** What turning this on actually changes, in one line. */ + description: string; + /** Who to ask. */ + owner: string; + /** + * ISO date after which this canary is overdue for removal. A canary that + * outlives its rollout is a permanent fork of the product with none of the + * review a permanent fork would have received. `assertNoOverdueCanaries` + * turns the date into a failing test rather than a good intention. + */ + sunsetAfter: string; +} + +export const CANARIES: readonly CanaryDefinition[] = [ + // ── Calibration ────────────────────────────────────────────────────────── + // Two INERT canaries that gate nothing. They exist to validate the rollout + // mechanism against real traffic before anything real depends on it, and + // they answer questions the synthetic tests cannot: + // + // 1. Does a requested percentage land on target in the wild? The unit + // tests use generated UUIDs and weight every install equally; real + // render volume is heavily skewed toward a few heavy installs, so the + // render-weighted share could differ from the install-weighted one. + // 2. How fast does CUMULATIVE exposure drift above the target? Install + // ids churn (measured: 24.7x more distinct ids over 30 days than in + // any single day), so the set of installs enrolled AT SOME POINT grows + // even though the instantaneous share stays flat. That drift is the + // real limit on a canary's blast-radius guarantee. + // 3. Are two canaries actually independent on real ids, not just on + // generated ones? Overlap should be ~p1*p2, not ~min(p1,p2). + // + // Two different percentages so the answer is a line, not a point. + // Delete both once the calibration window is read. + { + name: "calibration-10", + percentage: 10, + description: "Inert. Validates rollout accuracy and cumulative-exposure drift at 10%.", + owner: "vance", + sunsetAfter: "2026-09-15", + }, + { + name: "calibration-50", + percentage: 50, + description: + "Inert. Second calibration point, and an independence check against calibration-10.", + owner: "vance", + sunsetAfter: "2026-09-15", + }, + // ── Real rollouts ──────────────────────────────────────────────────────── + { + name: "de-parallel-router", + percentage: 0, + description: + "Route auto multi-worker renders to verified parallel drawElement streaming (HF_DE_PARALLEL_ROUTER). Ramp only alongside the per-install circuit breaker.", + owner: "vance", + sunsetAfter: "2026-10-01", + }, +] as const; + +export function findCanary(name: string): CanaryDefinition | undefined { + return CANARIES.find((c) => c.name === name); +} + +/** Env-var name for a feature's manual override: `de-parallel-router` → `HF_CANARY_DE_PARALLEL_ROUTER`. */ +export function canaryEnvVar(name: string): string { + return `HF_CANARY_${name.toUpperCase().replace(/[^A-Z0-9]+/g, "_")}`; +} + +/** + * Names of canaries whose sunset date has passed — either finish the rollout + * and delete the entry, or push the date with a reason. Exposed as a function + * (not a lint rule) so the check runs in the normal test suite. + */ +export function overdueCanaries(now: Date = new Date()): string[] { + return CANARIES.filter((c) => { + // End of the sunset day, not its start. The field is documented as the + // date AFTER which a canary is overdue, but comparing against midnight UTC + // made it overdue ON that date — and earlier still for anyone west of UTC. + const sunsetEnd = Date.parse(`${c.sunsetAfter}T23:59:59.999Z`); + return Number.isFinite(sunsetEnd) && now.getTime() > sunsetEnd; + }).map((c) => c.name); +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index b23ea509ff..8da2e39fe1 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -329,3 +329,19 @@ export { isBlockItem, isComponentItem, } from "./registry/index.js"; + +export { + canaryBucket, + evaluateCanary, + parseCanaryOverride, + type CanaryDecision, + type CanaryInput, + type CanaryReason, +} from "./canary.js"; +export { + CANARIES, + canaryEnvVar, + findCanary, + overdueCanaries, + type CanaryDefinition, +} from "./canaryRegistry.js"; diff --git a/packages/studio/src/telemetry/canary.test.ts b/packages/studio/src/telemetry/canary.test.ts new file mode 100644 index 0000000000..802a4aaf2b --- /dev/null +++ b/packages/studio/src/telemetry/canary.test.ts @@ -0,0 +1,325 @@ +// @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. +// The policy reads import.meta.env.DEV, which vitest sets true — without +// this every case would resolve to telemetry_opt_out. Controlled explicitly +// so each test states the privacy posture it is exercising. +const policyState = { allowed: true }; +vi.mock("./policy", () => ({ + browserTelemetryAllowed: () => policyState.allowed, +})); + +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, + canaryEventProperties, + canaryParamName, + __resetStudioCanaryCacheForTests, +} = await import("./canary"); +const { resolveStudioDistinctId, __resetStudioDistinctIdForTests } = await import("./distinctId"); + +function setSearch(search: string): void { + window.history.replaceState({}, "", `/${search}`); +} + +beforeEach(() => { + policyState.allowed = true; + localStorage.clear(); + sessionStorage.clear(); + setSearch(""); + delete window.__HF_CLI_DISTINCT_ID; + delete window.__HF_CLI_BUCKET_SEED; + 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 CLI's bucket seed when injected — the unit that survives config wipes", () => { + // The CLI buckets on its bucketSeed (inherited across config wipes via + // the install-state file), so a CLI-launched Studio must bucket on the + // SAME seed or the two surfaces would split one machine across cohorts. + const cliId = "db0c1f4a-b95e-4c35-90c6-1a15bd76f717"; + const cliSeed = "5f1c9d2e-0000-4000-8000-aaaaaaaaaaaa"; + window.__HF_CLI_DISTINCT_ID = cliId; + window.__HF_CLI_BUCKET_SEED = cliSeed; + __resetStudioDistinctIdForTests(); + __resetStudioCanaryCacheForTests(); + + // Telemetry identity still adopts the DISTINCT id — the seed only buckets. + expect(resolveStudioDistinctId()).toBe(cliId); + const viaBinding = resolveCanary("on-everywhere").bucket; + const bySeed = evaluateCanary({ + feature: "on-everywhere", + unitId: cliSeed, + percentage: 100, + }).bucket; + expect(viaBinding).toBe(bySeed); + }); + + it("buckets on the Studio distinct id when no seed is injected (standalone Studio)", () => { + const cliId = "db0c1f4a-b95e-4c35-90c6-1a15bd76f717"; + window.__HF_CLI_DISTINCT_ID = cliId; + __resetStudioDistinctIdForTests(); + __resetStudioCanaryCacheForTests(); + + expect(resolveStudioDistinctId()).toBe(cliId); + 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("emits the same PostHog flag-shaped properties as the CLI", () => { + expect(canaryEventProperties()).toEqual({ + "$feature/canary-on-everywhere": "true", + "$feature/canary-off-everywhere": "false", + }); + + __resetStudioCanaryCacheForTests(); + setSearch("?hf_canary_on_everywhere=off"); + expect(canaryEventProperties()["$feature/canary-on-everywhere"]).toBe("false"); + }); +}); + +describe("telemetry opt-out is canary opt-out", () => { + // The studio opt-out lever, per telemetry/config.ts. + const OPT_OUT_KEY = "hyperframes-studio:telemetryDisabled"; + + it("does not enrol an opted-out browser profile", () => { + policyState.allowed = false; + localStorage.setItem(OPT_OUT_KEY, "1"); + // on-everywhere is at 100% — it would be on for everyone otherwise. + expect(resolveCanary("on-everywhere")).toEqual({ + enabled: false, + reason: "telemetry_opt_out", + }); + }); + + it("never buckets an opted-out profile — no cohort is assigned at all", () => { + policyState.allowed = false; + localStorage.setItem(OPT_OUT_KEY, "1"); + expect(resolveCanary("on-everywhere").bucket).toBeUndefined(); + }); + + it("still honours an explicit URL override", () => { + policyState.allowed = false; + localStorage.setItem(OPT_OUT_KEY, "1"); + setSearch("?hf_canary_off_everywhere=on"); + expect(resolveCanary("off-everywhere")).toEqual({ enabled: true, reason: "forced_on" }); + }); + + it("reports every canary as false when opted out", () => { + policyState.allowed = false; + localStorage.setItem(OPT_OUT_KEY, "1"); + expect(canaryEventProperties()).toEqual({ + "$feature/canary-on-everywhere": "false", + "$feature/canary-off-everywhere": "false", + }); + }); +}); + +describe("CLI-launched Studio adopts the CLI's decisions", () => { + const OPT_OUT_KEY = "hyperframes-studio:telemetryDisabled"; + const cohort = (enabled: boolean) => ({ enabled, forced: false }); + const forced = (enabled: boolean) => ({ enabled, forced: true }); + + afterEach(() => { + delete window.__HF_CLI_CANARY_DECISIONS; + }); + + // The divergence this exists for: CLI telemetry off resolves every canary + // to telemetry_opt_out, but Studio's opt-out is a SEPARATE localStorage + // flag it cannot see — left to itself it would evaluate and could enrol. + it("stays off when the CLI opted out, even though Studio's own flag is unset", () => { + expect(localStorage.getItem(OPT_OUT_KEY)).toBeNull(); + window.__HF_CLI_CANARY_DECISIONS = { "on-everywhere": cohort(false) }; + expect(resolveCanary("on-everywhere").enabled).toBe(false); + }); + + // HF_CANARY_* never crosses into the browser, so before this the CLI was + // forced on and Studio silently guessed from the percentage. + it("turns on when the CLI forced it on, with no URL param present", () => { + window.__HF_CLI_CANARY_DECISIONS = { "off-everywhere": forced(true) }; + expect(resolveCanary("off-everywhere").enabled).toBe(true); + }); + + it("beats a contradicting URL override — one render must not run half-enrolled", () => { + window.__HF_CLI_CANARY_DECISIONS = { "on-everywhere": forced(false) }; + setSearch("?hf_canary_on_everywhere=on"); + expect(resolveCanary("on-everywhere").enabled).toBe(false); + }); + + it("beats the seed-derived bucket", () => { + window.__HF_CLI_BUCKET_SEED = "5f1c9d2e-0000-4000-8000-aaaaaaaaaaaa"; + window.__HF_CLI_CANARY_DECISIONS = { "on-everywhere": cohort(false) }; + expect(resolveCanary("on-everywhere").enabled).toBe(false); + }); + + it("falls back to local evaluation for a canary the CLI did not publish", () => { + window.__HF_CLI_CANARY_DECISIONS = { "off-everywhere": cohort(true) }; + expect(resolveCanary("on-everywhere").enabled).toBe(true); + }); + + it("ignores a malformed entry rather than trusting it", () => { + window.__HF_CLI_CANARY_DECISIONS = { + "on-everywhere": { enabled: "false" }, + } as unknown as Record; + // Falls through to local evaluation: on-everywhere is at 100%. + expect(resolveCanary("on-everywhere").enabled).toBe(true); + }); + + // Miguel's P1: a percentage roll from the CLI must NOT be able to enrol a + // browser profile that opted out. The two surfaces have independent + // opt-outs, and CLI telemetry being on says nothing about this profile. + describe("precedence against Studio's own opt-out", () => { + beforeEach(() => { + policyState.allowed = false; + localStorage.setItem(OPT_OUT_KEY, "1"); + }); + + it("refuses a CLI COHORT enrolment when this profile opted out", () => { + window.__HF_CLI_CANARY_DECISIONS = { "off-everywhere": cohort(true) }; + expect(resolveCanary("off-everywhere")).toEqual({ + enabled: false, + reason: "telemetry_opt_out", + }); + }); + + it("honours a CLI FORCED enrolment even when this profile opted out", () => { + // An explicit HF_CANARY_* override is a deliberate operator choice — + // the documented escalation channel, same as a local URL override. + window.__HF_CLI_CANARY_DECISIONS = { "off-everywhere": forced(true) }; + expect(resolveCanary("off-everywhere")).toEqual({ enabled: true, reason: "forced_on" }); + }); + + it("honours a CLI forced-OFF when this profile opted out", () => { + window.__HF_CLI_CANARY_DECISIONS = { "on-everywhere": forced(false) }; + expect(resolveCanary("on-everywhere")).toEqual({ enabled: false, reason: "forced_off" }); + }); + + it("still refuses cohort enrolment with no CLI decision at all", () => { + expect(resolveCanary("on-everywhere").reason).toBe("telemetry_opt_out"); + }); + }); +}); diff --git a/packages/studio/src/telemetry/canary.ts b/packages/studio/src/telemetry/canary.ts new file mode 100644 index 0000000000..d6240b2b78 --- /dev/null +++ b/packages/studio/src/telemetry/canary.ts @@ -0,0 +1,268 @@ +// --------------------------------------------------------------------------- +// 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 — the CLI's bucket seed (`window.__HF_CLI_BUCKET_SEED`) when the +// CLI launched Studio, so both surfaces bucket on the SAME unit and a +// rollout spanning render and editor is coherent for that user. Standalone +// Studio falls back to `resolveStudioDistinctId()` — the browser has no +// second storage location, so its localStorage id doubles as the seed. +// +// 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 { + canaryFeatureProperties, + evaluateCanary, + parseCanaryOverride, + type CanaryDecision, +} from "@hyperframes/core/canary"; +import { CANARIES, findCanary } from "@hyperframes/core/canary-registry"; +import { resolveStudioDistinctId } from "./distinctId"; +import { browserTelemetryAllowed } from "./policy"; +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, "_")}`; +} + +// Injected by the CLI's studio server alongside __HF_CLI_DISTINCT_ID. +declare global { + interface Window { + __HF_CLI_BUCKET_SEED?: string; + /** + * Resolved per-canary outcome from the launching CLI. `forced` marks an + * explicit `HF_CANARY_*` override as opposed to a percentage roll — the + * two get different precedence against Studio's own opt-out. + */ + __HF_CLI_CANARY_DECISIONS?: Record; + } +} + +/** + * The launching CLI's decision for this canary, if it published one. + * + * When present this WINS over everything Studio could work out locally — + * seed, URL override, registry percentage. Studio re-deriving cannot agree + * with the CLI in the cases that matter most: + * + * - telemetry off: the CLI resolves `telemetry_opt_out`, but Studio's + * opt-out is a separate localStorage flag it cannot see from here; + * - `HF_CANARY_*` override: env vars never cross into the browser; + * - no seed injected: Studio falls back to a different unit, i.e. a + * different bucket. + * + * In all three the CLI has already decided, and one render spanning both + * surfaces must not run half-enrolled. + */ +function cliDecision(name: string): { enabled: boolean; forced: boolean } | undefined { + try { + if (typeof window === "undefined") return undefined; + const decision = window.__HF_CLI_CANARY_DECISIONS?.[name]; + if (typeof decision?.enabled !== "boolean") return undefined; + return { enabled: decision.enabled, forced: decision.forced === true }; + } catch { + return undefined; + } +} + +/** + * The bucketing unit. A CLI-launched Studio buckets on the CLI's SEED (which + * survives config wipes via the install-state file), not its distinct id — + * the CLI itself buckets on the seed, and the two surfaces must agree per + * machine. Standalone Studio falls back to its own distinct id: the browser + * has no second storage location, so localStorage IS both id and seed there. + */ +function resolveBucketUnit(): string { + try { + const seed = typeof window === "undefined" ? undefined : window.__HF_CLI_BUCKET_SEED; + if (typeof seed === "string" && seed.length > 0) return seed; + } catch { + /* fall through */ + } + return resolveStudioDistinctId(); +} + +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(); +} + +/** The uncached decision. Split out so `resolveCanary` is purely the memo. */ +const forcedOutcome = (enabled: boolean): CanaryDecision => ({ + enabled, + reason: enabled ? "forced_on" : "forced_off", +}); + +const cohortOutcome = (enabled: boolean): CanaryDecision => ({ + enabled, + reason: enabled ? "in_cohort" : "out_of_cohort", +}); + +/** + * Precedence, highest first: + * + * 1. An explicit `HF_CANARY_*` from the launching CLI. A deliberate operator + * choice — the documented escalation channel (dogfooding, bisect, + * panic-off) — so it wins outright, including over this profile's + * opt-out, exactly as a local URL override does. + * 2. A local URL / sessionStorage override, same reasoning. + * 3. This profile's telemetry opt-out. Checked BEFORE any percentage-derived + * CLI decision: the two surfaces have independent opt-outs, and CLI + * telemetry being on says nothing about whether THIS browser profile + * agreed to be measured. A cohort roll must never enrol an opted-out + * profile. + * 4. The CLI's percentage decision — authoritative, since it already applied + * the shared seed and re-deriving is what let the surfaces disagree. + * 5. Local evaluation (standalone Studio, or a canary the CLI didn't publish). + */ +function decideStudioCanary(name: string): CanaryDecision { + const definition = findCanary(name); + if (!definition) return { enabled: false, reason: "out_of_cohort" }; + + const fromCli = cliDecision(definition.name); + if (fromCli?.forced) return forcedOutcome(fromCli.enabled); + + const override = readOverride(definition.name); + if (override === undefined) { + if (!browserTelemetryAllowed()) return { enabled: false, reason: "telemetry_opt_out" }; + // Studio's exclusion is its own to apply: the CLI cannot see + // navigator.webdriver, so adopting its cohort decision verbatim enrolled + // Playwright/Puppeteer sessions driving a local `hyperframes preview` — + // each minting a fresh localStorage id, precisely the ephemeral-id noise + // the exclusion exists to keep out of the rollout signal. + if (isAutomatedBrowser()) return { enabled: false, reason: "excluded" }; + if (fromCli !== undefined) return cohortOutcome(fromCli.enabled); + } + + // An explicit override decides on evaluateCanary's first line without ever + // reading unitId, so resolving the unit here would mint and PERSIST an + // anonymous id purely as an unused argument — for a profile that may have + // opted out. One click on a support link created a durable tracking id. + if (override !== undefined) return forcedOutcome(override); + + return evaluateCanary({ + feature: definition.name, + unitId: resolveBucketUnit(), + percentage: definition.percentage, + exclude: isAutomatedBrowser(), + }); +} + +/** + * 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 decision = decideStudioCanary(name); + + 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; +} + +/** + * Canary assignments as PostHog flag properties (`$feature/canary-`), + * attached to every Studio event so any metric can be split by cohort — + * identical shape to the CLI, so a rollout spanning both reads as one flag. + */ +export function canaryEventProperties(): Record { + return canaryFeatureProperties( + CANARIES.map((c) => ({ name: c.name, enabled: resolveCanary(c.name).enabled })), + ); +} diff --git a/packages/studio/src/telemetry/client.test.ts b/packages/studio/src/telemetry/client.test.ts index 079bbe4966..85602bb74b 100644 --- a/packages/studio/src/telemetry/client.test.ts +++ b/packages/studio/src/telemetry/client.test.ts @@ -77,10 +77,15 @@ describe("studio client shouldTrack", () => { expect(shouldTrack()).toBe(false); }); - it("memoizes its decision after the first call", async () => { + // Previously asserted the opposite. That memoization WAS the bug: policy.ts + // is explicit that transports re-ask, and policy.test.ts asserts a + // mid-session opt-out takes effect at once — but this transport cached on + // first call, so a user who opted out in DevTools after one event kept + // sending `studio_*` and render events while `studio:*` correctly stopped. + it("re-reads the policy, so a mid-session opt-out takes effect immediately", async () => { const shouldTrack = await loadShouldTrack(); - const first = shouldTrack(); + expect(shouldTrack()).toBe(true); localStorage.setItem(OPT_OUT_KEY, "1"); - expect(shouldTrack()).toBe(first); + expect(shouldTrack()).toBe(false); }); }); diff --git a/packages/studio/src/telemetry/client.ts b/packages/studio/src/telemetry/client.ts index cdeccfb454..615d497d31 100644 --- a/packages/studio/src/telemetry/client.ts +++ b/packages/studio/src/telemetry/client.ts @@ -4,8 +4,10 @@ // All calls are fire-and-forget; telemetry must never break the studio UI. // --------------------------------------------------------------------------- -import { getAnonymousId, hasShownNotice, isOptedOut, markNoticeShown } from "./config"; +import { getAnonymousId, hasShownNotice, markNoticeShown } from "./config"; +import { browserTelemetryAllowed } from "./policy"; import { getBrowserSystemMeta } from "./system"; +import { canaryEventProperties } from "./canary"; // Write-only PostHog project key, safe to embed in client code. const POSTHOG_API_KEY = "phc_zjjbX0PnWxERXrMHhkEJWj9A9BhGVLRReICgsfTMmpx"; @@ -22,49 +24,14 @@ interface QueuedEvent { let eventQueue: QueuedEvent[] = []; let flushTimer: ReturnType | null = null; -let telemetryEnabled: boolean | null = null; - -function isDoNotTrackOn(): boolean { - return typeof navigator !== "undefined" && navigator.doNotTrack === "1"; -} - -function isApiKeyConfigured(): boolean { - return POSTHOG_API_KEY.startsWith("phc_"); -} - -// VITE_HYPERFRAMES_NO_TELEMETRY mirrors the CLI's HYPERFRAMES_NO_TELEMETRY=1 -// opt-out so HeyGen's own dev/CI builds can suppress telemetry from the studio -// bundle the same way. Vite injects it at build time. Match the CLI's -// affirmative privacy-control spellings. -// `import.meta.env` may be undefined in non-Vite bundlers (Next.js Turbopack). -function isBuildTimeOptOut(): boolean { - try { - const v = import.meta.env.VITE_HYPERFRAMES_NO_TELEMETRY as string | undefined; - return v !== undefined && ["1", "true", "yes", "on"].includes(v.trim().toLowerCase()); - } catch { - return false; - } -} - -// `import.meta.env.DEV` is true under `vite dev` / `vite preview`. Auto-suppress -// so developers running `hyperframes preview` don't pollute production telemetry. -function isViteDevMode(): boolean { - try { - return import.meta.env.DEV === true; - } catch { - return false; - } -} export function shouldTrack(): boolean { - if (telemetryEnabled !== null) return telemetryEnabled; - telemetryEnabled = - isApiKeyConfigured() && - !isBuildTimeOptOut() && - !isViteDevMode() && - !isOptedOut() && - !isDoNotTrackOn(); - return telemetryEnabled; + // NOT memoized. policy.ts is explicit that the transports re-ask, and + // policy.test.ts asserts a mid-session opt-out takes effect at once — but + // this cached on first call, so a user who opted out in DevTools after one + // event kept sending `studio_*` and render events for the rest of the tab + // while `studio:*` correctly stopped. The check is two property reads. + return browserTelemetryAllowed(); } export function trackEvent(event: string, properties: EventProperties = {}): void { @@ -73,7 +40,10 @@ export function trackEvent(event: string, properties: EventProperties = {}): voi const sys = getBrowserSystemMeta(); eventQueue.push({ event, - properties: { ...properties, ...sys }, + // Canary assignments as `$feature/canary-`, mirroring the CLI so a + // rollout spanning both surfaces reads as one flag in PostHog. Resolved + // after the shouldTrack guard, so opted-out users never pay for it. + properties: { ...properties, ...sys, ...canaryEventProperties() }, timestamp: new Date().toISOString(), }); diff --git a/packages/studio/src/telemetry/config.ts b/packages/studio/src/telemetry/config.ts index f4326b97b0..970f5b136f 100644 --- a/packages/studio/src/telemetry/config.ts +++ b/packages/studio/src/telemetry/config.ts @@ -22,12 +22,26 @@ export function getAnonymousId(): string { return resolveStudioDistinctId(); } +// safeLocalStorage() guards the REFERENCE, not the access: in a partitioned +// or sandboxed context the object resolves and `getItem` still throws (the +// case distinctId.ts already documents). These are read from the telemetry +// policy, which is called from event tracking that must never throw into a +// caller — `trackStudioEvent` sits in a post-commit catch block, so a throw +// there reported an already-committed edit as failed. +function readStoredFlag(key: string): boolean { + try { + return safeLocalStorage()?.getItem(key) === "1"; + } catch { + return false; + } +} + export function isOptedOut(): boolean { - return safeLocalStorage()?.getItem(OPT_OUT_KEY) === "1"; + return readStoredFlag(OPT_OUT_KEY); } export function hasShownNotice(): boolean { - return safeLocalStorage()?.getItem(NOTICE_KEY) === "1"; + return readStoredFlag(NOTICE_KEY); } export function markNoticeShown(): void { diff --git a/packages/studio/src/telemetry/distinctId.test.ts b/packages/studio/src/telemetry/distinctId.test.ts index b420114639..b6e77119be 100644 --- a/packages/studio/src/telemetry/distinctId.test.ts +++ b/packages/studio/src/telemetry/distinctId.test.ts @@ -101,3 +101,38 @@ describe("getCliDistinctId", () => { expect(getCliDistinctId()).toBeNull(); }); }); + +describe("no-storage fallback must not collapse the population", () => { + const realLocalStorage = Object.getOwnPropertyDescriptor(globalThis, "localStorage"); + + beforeEach(() => { + __resetStudioDistinctIdForTests(); + // Simulate a hardened / partitioned context where safeLocalStorage() + // returns null. + Object.defineProperty(globalThis, "localStorage", { value: undefined, configurable: true }); + }); + + afterEach(() => { + if (realLocalStorage) Object.defineProperty(globalThis, "localStorage", realLocalStorage); + __resetStudioDistinctIdForTests(); + }); + + it("is stable within a session", () => { + const first = resolveStudioDistinctId(); + expect(resolveStudioDistinctId()).toBe(first); + expect(first).toBeTruthy(); + }); + + // Regression: this used to return the shared literal "anonymous", so every + // storage-restricted profile was ONE bucketing unit. Against the shipped + // hash `calibration-50:anonymous` lands in bucket 44, so that whole + // population was enrolled at a nominal 50% and would flip together on a + // ramp — and they all merged into one PostHog person. + it("differs across sessions rather than sharing one constant", () => { + const first = resolveStudioDistinctId(); + __resetStudioDistinctIdForTests(); + const second = resolveStudioDistinctId(); + expect(second).not.toBe(first); + expect(first).not.toBe("anonymous"); + }); +}); diff --git a/packages/studio/src/telemetry/distinctId.ts b/packages/studio/src/telemetry/distinctId.ts index 241b9225ac..790beaf888 100644 --- a/packages/studio/src/telemetry/distinctId.ts +++ b/packages/studio/src/telemetry/distinctId.ts @@ -106,9 +106,18 @@ export function resolveStudioDistinctId(): string { return existing; } } else { - // No storage at all (SSR / locked-down browser): stable within the session. + // No storage at all (SSR / locked-down browser): stable within the + // session, but RANDOM per session rather than a shared literal. + // + // It used to be the constant "anonymous", which made every + // storage-restricted profile one bucketing unit: computed against the + // shipped hash, `calibration-50:anonymous` lands in bucket 44, so 100% of + // that population was enrolled at a nominal 50% and would flip together + // on any ramp. It also merged unrelated users into a single PostHog + // person. A per-session id spreads them and keeps them distinct, while + // persisting nothing. // `cachedId` is guaranteed null here (early-returned at the top otherwise). - cachedId = "anonymous"; + cachedId = generateId(); return cachedId; } diff --git a/packages/studio/src/telemetry/policy.test.ts b/packages/studio/src/telemetry/policy.test.ts new file mode 100644 index 0000000000..e11bbbfaec --- /dev/null +++ b/packages/studio/src/telemetry/policy.test.ts @@ -0,0 +1,81 @@ +// @vitest-environment happy-dom +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +// Each control the browser telemetry policy enforces, asserted individually. +// This is the SSOT that `telemetry/client.ts`, `utils/studioTelemetry.ts` and +// canary enrolment all consult, so a gap here is a gap in all three — which is +// how `navigator.doNotTrack` and Vite dev mode came to suppress one transport +// but not the other, nor enrolment. + +const DOCUMENTED_OPT_OUT = "hyperframes-studio:telemetryDisabled"; +const LEGACY_OPT_OUT = "hf-studio-telemetry-opt-out"; + +describe("browserTelemetryAllowed", () => { + let browserTelemetryAllowed: typeof import("./policy").browserTelemetryAllowed; + + beforeEach(async () => { + localStorage.clear(); + vi.resetModules(); + // vitest sets import.meta.env.DEV; the policy suppresses under it, so the + // baseline has to be an explicitly production-like env. + vi.stubEnv("DEV", false); + vi.stubEnv("VITE_HYPERFRAMES_NO_TELEMETRY", ""); + Object.defineProperty(navigator, "doNotTrack", { value: null, configurable: true }); + ({ browserTelemetryAllowed } = await import("./policy")); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + }); + + it("allows telemetry with no control set", () => { + expect(browserTelemetryAllowed()).toBe(true); + }); + + it("refuses when the documented localStorage key is set", () => { + localStorage.setItem(DOCUMENTED_OPT_OUT, "1"); + expect(browserTelemetryAllowed()).toBe(false); + }); + + // Anyone already opted out this way must never be quietly re-enabled by the + // move to the documented key. + it("refuses when the legacy localStorage key is set", () => { + localStorage.setItem(LEGACY_OPT_OUT, "1"); + expect(browserTelemetryAllowed()).toBe(false); + }); + + it("refuses when navigator.doNotTrack is on", () => { + Object.defineProperty(navigator, "doNotTrack", { value: "1", configurable: true }); + expect(browserTelemetryAllowed()).toBe(false); + }); + + it("refuses under Vite dev mode", async () => { + vi.stubEnv("DEV", true); + vi.resetModules(); + ({ browserTelemetryAllowed } = await import("./policy")); + expect(browserTelemetryAllowed()).toBe(false); + }); + + it.each(["1", "true", "yes", "on", " ON "])( + "refuses when VITE_HYPERFRAMES_NO_TELEMETRY=%s", + async (value) => { + vi.stubEnv("VITE_HYPERFRAMES_NO_TELEMETRY", value); + vi.resetModules(); + ({ browserTelemetryAllowed } = await import("./policy")); + expect(browserTelemetryAllowed()).toBe(false); + }, + ); + + it("ignores an unset or unrelated VITE_HYPERFRAMES_NO_TELEMETRY value", async () => { + vi.stubEnv("VITE_HYPERFRAMES_NO_TELEMETRY", "0"); + vi.resetModules(); + ({ browserTelemetryAllowed } = await import("./policy")); + expect(browserTelemetryAllowed()).toBe(true); + }); + + it("is not memoized — a mid-session opt-out takes effect immediately", () => { + expect(browserTelemetryAllowed()).toBe(true); + localStorage.setItem(DOCUMENTED_OPT_OUT, "1"); + expect(browserTelemetryAllowed()).toBe(false); + }); +}); diff --git a/packages/studio/src/telemetry/policy.ts b/packages/studio/src/telemetry/policy.ts new file mode 100644 index 0000000000..9cf123dae5 --- /dev/null +++ b/packages/studio/src/telemetry/policy.ts @@ -0,0 +1,104 @@ +// --------------------------------------------------------------------------- +// Browser telemetry policy — the single answer to "may this profile be +// measured?", shared by every transport and by canary enrolment. +// +// This exists because the answer was previously duplicated and the copies had +// drifted: `telemetry/client.ts` enforced five controls, the older +// `utils/studioTelemetry.ts` transport enforced one (its own localStorage +// key), and canary evaluation enforced a different one. So a profile with +// `navigator.doNotTrack` set, or a Vite dev build, still emitted `studio:*` +// events AND could be bucketed into a rollout — under controls the public +// docs say disable both. +// +// Deliberately imports only `./config` (localStorage helpers). Nothing here +// may import a transport or the canary module: both of those import this, and +// the whole point is one definition with no cycle. +// --------------------------------------------------------------------------- + +import { isOptedOut } from "./config"; + +// Write-only PostHog project key, safe to embed in client code. Duplicated +// from client.ts intentionally — the eligibility check must not drag the +// transport (and its queue/timer state) into modules that only need the +// policy. +const POSTHOG_API_KEY = "phc_zjjbX0PnWxERXrMHhkEJWj9A9BhGVLRReICgsfTMmpx"; + +/** Legacy opt-out key predating `telemetry/config.ts`. Still honoured so + * anyone already opted out is never quietly re-enabled. */ +const LEGACY_OPT_OUT_KEY = "hf-studio-telemetry-opt-out"; + +function isLegacyOptedOut(): boolean { + try { + return localStorage.getItem(LEGACY_OPT_OUT_KEY) === "1"; + } catch { + return false; + } +} + +function isDoNotTrackOn(): boolean { + return typeof navigator !== "undefined" && navigator.doNotTrack === "1"; +} + +function isApiKeyConfigured(): boolean { + return POSTHOG_API_KEY.startsWith("phc_"); +} + +// VITE_HYPERFRAMES_NO_TELEMETRY mirrors the CLI's HYPERFRAMES_NO_TELEMETRY=1 +// opt-out so HeyGen's own dev/CI builds can suppress telemetry from the studio +// bundle the same way. Vite injects it at build time. Match the CLI's +// affirmative privacy-control spellings. +// `import.meta.env` may be undefined in non-Vite bundlers (Next.js Turbopack). +function isBuildTimeOptOut(): boolean { + try { + const v = import.meta.env.VITE_HYPERFRAMES_NO_TELEMETRY as string | undefined; + return v !== undefined && ["1", "true", "yes", "on"].includes(v.trim().toLowerCase()); + } catch { + return false; + } +} + +// `import.meta.env.DEV` is true under `vite dev` / `vite preview`. Auto-suppress +// so developers running `hyperframes preview` don't pollute production telemetry. +function isViteDevMode(): boolean { + try { + return import.meta.env.DEV === true; + } catch { + return false; + } +} + +/** + * May this browser profile be measured at all? + * + * Governs BOTH sending events and canary enrolment. Enrolment is part of + * measurement, not separate from it: a profile that reports nothing cannot be + * compared against anyone, so bucketing it changes that user's code path for + * no signal. The one documented exception is an explicit `HF_CANARY_*` / + * `?hf_canary_*=` override, which callers apply before consulting this. + * + * Not memoized — `isOptedOut()` reads localStorage, which a user can flip in + * DevTools mid-session, and the per-call cost is a couple of property reads. + * Callers that must stay stable within a session memoize their own result + * (canary decisions do; the transports intentionally do not). + */ +export function browserTelemetryAllowed(): boolean { + try { + return allowed(); + } catch { + // Fail CLOSED. A storage read that throws must not enrol anyone, and must + // not propagate: callers include a post-commit catch block where a throw + // reports a committed edit as failed. + return false; + } +} + +function allowed(): boolean { + return ( + isApiKeyConfigured() && + !isBuildTimeOptOut() && + !isViteDevMode() && + !isOptedOut() && + !isLegacyOptedOut() && + !isDoNotTrackOn() + ); +} diff --git a/packages/studio/src/utils/studioTelemetry.test.ts b/packages/studio/src/utils/studioTelemetry.test.ts new file mode 100644 index 0000000000..b7eb42cf60 --- /dev/null +++ b/packages/studio/src/utils/studioTelemetry.test.ts @@ -0,0 +1,76 @@ +// @vitest-environment happy-dom +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +// The `studio:*` path predates telemetry/config.ts and shipped its own +// opt-out key and its own send loop, so it sat outside both contracts the +// canary work established: the documented opt-out did not silence it, and its +// events carried no cohort assignment. These pin both. + +vi.mock("../telemetry/canary", () => ({ + canaryEventProperties: () => ({ "$feature/canary-test-one": "true" }), +})); + +// One shared policy now governs this transport, telemetry/client.ts and +// canary enrolment. Exercised directly here so each case names the control +// under test rather than relying on ambient import.meta.env. +const policyState = { allowed: true }; +vi.mock("../telemetry/policy", () => ({ + browserTelemetryAllowed: () => policyState.allowed, +})); + +describe("studioTelemetry — shared opt-out and canary properties", () => { + let trackStudioEvent: typeof import("./studioTelemetry").trackStudioEvent; + let fetchMock: ReturnType; + + beforeEach(async () => { + policyState.allowed = true; + localStorage.clear(); + vi.resetModules(); + vi.useFakeTimers(); + fetchMock = vi.fn(() => Promise.resolve({ ok: true } as Response)); + vi.stubGlobal("fetch", fetchMock); + ({ trackStudioEvent } = await import("./studioTelemetry")); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.unstubAllGlobals(); + }); + + /** Drain the queue and return the events the batch would have sent. */ + async function sentEvents(): Promise>> { + await vi.runOnlyPendingTimersAsync(); + if (fetchMock.mock.calls.length === 0) return []; + const body = fetchMock.mock.calls[0]?.[1] as { body?: string } | undefined; + const parsed = JSON.parse(body?.body ?? "{}") as { batch?: Array> }; + return parsed.batch ?? []; + } + + // Every control the shared policy enforces — documented key, legacy key, + // navigator.doNotTrack, VITE_HYPERFRAMES_NO_TELEMETRY, Vite dev mode, API + // key eligibility. Before the policy was shared this transport honoured + // only the legacy key, so all of the others still emitted `studio:*`. + it("sends nothing when the shared policy refuses", async () => { + policyState.allowed = false; + trackStudioEvent("thing_happened"); + expect(await sentEvents()).toHaveLength(0); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("attaches canary assignments to every event", async () => { + trackStudioEvent("thing_happened"); + const events = await sentEvents(); + expect(events).toHaveLength(1); + expect(events[0]?.["properties"]).toMatchObject({ + "$feature/canary-test-one": "true", + }); + }); + + it("lets an explicit property win over the canary mixin", async () => { + trackStudioEvent("thing_happened", { "$feature/canary-test-one": "false" }); + const events = await sentEvents(); + expect(events[0]?.["properties"]).toMatchObject({ + "$feature/canary-test-one": "false", + }); + }); +}); diff --git a/packages/studio/src/utils/studioTelemetry.ts b/packages/studio/src/utils/studioTelemetry.ts index c73add7afc..373127d0ce 100644 --- a/packages/studio/src/utils/studioTelemetry.ts +++ b/packages/studio/src/utils/studioTelemetry.ts @@ -1,4 +1,6 @@ import { resolveStudioDistinctId } from "../telemetry/distinctId"; +import { browserTelemetryAllowed } from "../telemetry/policy"; +import { canaryEventProperties } from "../telemetry/canary"; // PostHog public ingest key — write-only, safe to ship in the client bundle const POSTHOG_API_KEY = "phc_zjjbX0PnWxERXrMHhkEJWj9A9BhGVLRReICgsfTMmpx"; @@ -26,12 +28,15 @@ function getDistinctId(): string { return resolveStudioDistinctId(); } +/** + * This path predates telemetry/config.ts and enforced only its own + * localStorage key, so `navigator.doNotTrack`, VITE_HYPERFRAMES_NO_TELEMETRY, + * Vite dev mode and the documented `hyperframes-studio:telemetryDisabled` all + * failed to silence `studio:*` events. Now one shared policy governs every + * transport — including the legacy key, which it still honours. + */ function isEnabled(): boolean { - try { - return localStorage.getItem("hf-studio-telemetry-opt-out") !== "1"; - } catch { - return true; - } + return browserTelemetryAllowed(); } function getSessionProperties(): EventProperties { @@ -56,7 +61,10 @@ export function trackStudioEvent(event: string, properties: EventProperties = {} queue.push({ event: `studio:${event}`, - properties: { ...getSessionProperties(), ...properties }, + // Canary assignments on every event, matching the CLI and the newer + // studio client — "every telemetry event carries the assignment" has to + // include this path or a cohort breakdown silently omits `studio:*`. + properties: { ...getSessionProperties(), ...canaryEventProperties(), ...properties }, timestamp: new Date().toISOString(), });