diff --git a/docs/contributing/canary-rollouts.mdx b/docs/contributing/canary-rollouts.mdx index ea6f15386..58981a4db 100644 --- a/docs/contributing/canary-rollouts.mdx +++ b/docs/contributing/canary-rollouts.mdx @@ -206,8 +206,22 @@ 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, never emitted, and shared with a CLI-launched Studio so both -surfaces agree. It does not outlive `~/.hyperframes`. +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 plain +`{ name: boolean }` map — and Studio takes it as authoritative over its own +seed, URL override and the registry percentage. 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. + +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 diff --git a/packages/cli/src/server/telemetryIdentity.test.ts b/packages/cli/src/server/telemetryIdentity.test.ts index cd5535bd2..2791a3169 100644 --- a/packages/cli/src/server/telemetryIdentity.test.ts +++ b/packages/cli/src/server/telemetryIdentity.test.ts @@ -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,6 +16,9 @@ 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"); @@ -21,6 +27,8 @@ describe("resolveCliTelemetryDistinctId", () => { beforeEach(() => { shouldTrack.mockReset(); readConfig.mockReset(); + canaryDecisions.mockReset(); + canaryDecisions.mockReturnValue({}); }); it("returns the CLI anonymousId when telemetry is enabled", () => { @@ -56,6 +64,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", () => { @@ -74,11 +84,56 @@ describe("buildCliIdentityScript", () => { ); }); - it("emits an empty string when telemetry is disabled (nothing to seed)", () => { + 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": 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": 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: ""; @@ -105,7 +162,7 @@ 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); }); diff --git a/packages/cli/src/server/telemetryIdentity.ts b/packages/cli/src/server/telemetryIdentity.ts index 9a24765e4..c9d843b1e 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 } from "../telemetry/canary.js"; /** * The CLI's anonymous distinct id to hand to Studio, or null when CLI telemetry @@ -34,13 +35,6 @@ export function resolveCliTelemetryDistinctId(): string | 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 ? "" : ``; } /** diff --git a/packages/cli/src/telemetry/canary.ts b/packages/cli/src/telemetry/canary.ts index d76708d80..f7684a7c5 100644 --- a/packages/cli/src/telemetry/canary.ts +++ b/packages/cli/src/telemetry/canary.ts @@ -118,6 +118,32 @@ export function isCanaryEnabled(name: string): boolean { return resolveCanary(name).enabled; } +/** + * 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) out[canary.name] = resolveCanary(canary.name).enabled; + return out; +} + /** * Canary assignments as PostHog flag properties — `$feature/canary-` * set to `"true"` / `"false"` for every registered canary. Spread onto every diff --git a/packages/studio/src/telemetry/canary.test.ts b/packages/studio/src/telemetry/canary.test.ts index ff89f6a97..ab7b9d75c 100644 --- a/packages/studio/src/telemetry/canary.test.ts +++ b/packages/studio/src/telemetry/canary.test.ts @@ -225,3 +225,53 @@ describe("telemetry opt-out is canary opt-out", () => { }); }); }); + +describe("CLI-launched Studio adopts the CLI's decisions", () => { + const OPT_OUT_KEY = "hyperframes-studio:telemetryDisabled"; + + 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": 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": 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": 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": 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": true }; + expect(resolveCanary("on-everywhere").enabled).toBe(true); + }); + + it("ignores a non-boolean value rather than trusting it", () => { + window.__HF_CLI_CANARY_DECISIONS = { "on-everywhere": "false" } as unknown as Record< + string, + boolean + >; + // Falls through to local evaluation: on-everywhere is at 100%. + expect(resolveCanary("on-everywhere").enabled).toBe(true); + }); +}); diff --git a/packages/studio/src/telemetry/canary.ts b/packages/studio/src/telemetry/canary.ts index 943ed720f..9572633e1 100644 --- a/packages/studio/src/telemetry/canary.ts +++ b/packages/studio/src/telemetry/canary.ts @@ -53,6 +53,34 @@ export function canaryParamName(name: string): string { declare global { interface Window { __HF_CLI_BUCKET_SEED?: string; + /** Resolved on/off per canary from the launching CLI. Authoritative. */ + __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): boolean | undefined { + try { + if (typeof window === "undefined") return undefined; + const decision = window.__HF_CLI_CANARY_DECISIONS?.[name]; + return typeof decision === "boolean" ? decision : undefined; + } catch { + return undefined; } } @@ -142,6 +170,39 @@ export function __resetStudioCanaryCacheForTests(): void { decisions.clear(); } +/** The uncached decision. Split out so `resolveCanary` is purely the memo. */ +function decideStudioCanary(name: string): CanaryDecision { + const definition = findCanary(name); + if (!definition) return { enabled: false, reason: "out_of_cohort" }; + + // The launching CLI's decision, when there is one, is the whole answer: + // it already applied telemetry opt-out, HF_CANARY_* overrides and the + // percentage against the shared seed. Re-deriving here is what let the two + // surfaces disagree on the same render. + const fromCli = cliDecision(definition.name); + if (fromCli !== undefined) { + return { enabled: fromCli, reason: fromCli ? "forced_on" : "forced_off" }; + } + + const override = readOverride(definition.name); + // Standalone Studio. Opting out of telemetry opts you out of canaries — + // same rule as the CLI (see packages/cli/src/telemetry/canary.ts). A + // profile that sends nothing can't be compared against anyone, so + // enrolling it changes that user's code path for no signal. An explicit + // override still wins: a deliberate local choice, not silent enrolment. + if (override === undefined && isOptedOut()) { + return { enabled: false, reason: "telemetry_opt_out" }; + } + + return evaluateCanary({ + feature: definition.name, + unitId: resolveBucketUnit(), + percentage: definition.percentage, + override, + 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 @@ -151,24 +212,7 @@ export function resolveCanary(name: string): CanaryDecision { const cached = decisions.get(name); if (cached) return cached; - const definition = findCanary(name); - const override = definition ? readOverride(definition.name) : undefined; - // Opting out of telemetry opts you out of canaries — same rule as the CLI - // (see packages/cli/src/telemetry/canary.ts). An install that sends nothing - // can't be compared against anyone, so enrolling it changes that user's - // code path for no signal. An explicit override still wins: that is a - // deliberate local choice, not silent enrolment. - const decision: CanaryDecision = !definition - ? { enabled: false, reason: "out_of_cohort" } - : override === undefined && isOptedOut() - ? { enabled: false, reason: "telemetry_opt_out" } - : evaluateCanary({ - feature: definition.name, - unitId: resolveBucketUnit(), - percentage: definition.percentage, - override, - exclude: isAutomatedBrowser(), - }); + const decision = decideStudioCanary(name); decisions.set(name, decision); return decision;