mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-10 22:20:14 +00:00
fix(cli,studio): close the four R3 blocking gaps
P1 — SPA route bypassed the DNS-rebinding guard. Guarding only
/api/telemetry-identity left the catch-all as an open side door: a
rebound origin could fetch `/` and read __HF_CLI_DISTINCT_ID and
__HF_CLI_BUCKET_SEED straight out of the returned HTML. The SPA response
now applies the same isLoopbackHost() check; an untrusted Host still gets
a working Studio, just with no identity, seed or decisions injected.
Route-level regression added.
P1 — a CLI cohort roll could override Studio's own opt-out.
decideStudioCanary() adopted the injected decision before checking
isOptedOut(), so CLI-telemetry-on plus Studio-opted-out still enrolled
Studio. A bare boolean could not express the difference between a
deliberate override and an ordinary cohort roll, so the injected map now
carries provenance ({ enabled, forced }). Forced wins outright — it is
the documented escalation channel and must behave the same on both
surfaces — while a percentage roll now loses to this profile's opt-out.
Full interaction matrix tested.
P1 — the legacy studio:* path sat outside both contracts.
utils/studioTelemetry.ts shipped its own opt-out key and its own send
loop, so the documented hyperframes-studio:telemetryDisabled did not
silence it and its events carried no cohort assignment. It now honours
both keys (the legacy one stays, so nobody already opted out is quietly
re-enabled) and mixes in canaryEventProperties(), making "every
telemetry event carries the assignment" actually true.
P2 — partial salvage could drop a tripped breaker.
salvageInstallState() discarded the whole record when markerAt and
bucketSeed were both unusable, taking deParallelRouterTrialFired with it
and re-enrolling a machine whose router already failed. All three fields
are now independently salvageable.
Docs: canary-rollouts.mdx said "disabling telemetry disables the
reporting, not the enrolment" — exactly backwards since the opt-out gate
landed. Corrected; checked for other copies, none.
Tests: 13 new (4 opt-out precedence, 4 legacy-path opt-out and canary
props, 3 route-level host guard, 2 breaker salvage). Fault injection:
each of the four fixes reverted independently fails its own tests
(2 CLI + 1 Studio + 2 Studio).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
54534d53c2
commit
f81ab0162e
@@ -228,6 +228,8 @@ describe("telemetry opt-out is canary opt-out", () => {
|
||||
|
||||
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;
|
||||
@@ -238,40 +240,72 @@ describe("CLI-launched Studio adopts the CLI's decisions", () => {
|
||||
// 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 };
|
||||
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": true };
|
||||
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": false };
|
||||
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": false };
|
||||
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": true };
|
||||
window.__HF_CLI_CANARY_DECISIONS = { "off-everywhere": cohort(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
|
||||
>;
|
||||
it("ignores a malformed entry rather than trusting it", () => {
|
||||
window.__HF_CLI_CANARY_DECISIONS = {
|
||||
"on-everywhere": { enabled: "false" },
|
||||
} as unknown as Record<string, { enabled?: boolean; forced?: boolean }>;
|
||||
// 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(() => {
|
||||
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");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -53,8 +53,12 @@ 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<string, boolean>;
|
||||
/**
|
||||
* 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<string, { enabled?: boolean; forced?: boolean }>;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -74,11 +78,12 @@ declare global {
|
||||
* 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 {
|
||||
function cliDecision(name: string): { enabled: boolean; forced: boolean } | undefined {
|
||||
try {
|
||||
if (typeof window === "undefined") return undefined;
|
||||
const decision = window.__HF_CLI_CANARY_DECISIONS?.[name];
|
||||
return typeof decision === "boolean" ? decision : undefined;
|
||||
if (typeof decision?.enabled !== "boolean") return undefined;
|
||||
return { enabled: decision.enabled, forced: decision.forced === true };
|
||||
} catch {
|
||||
return undefined;
|
||||
}
|
||||
@@ -171,27 +176,44 @@ export function __resetStudioCanaryCacheForTests(): void {
|
||||
}
|
||||
|
||||
/** 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" };
|
||||
|
||||
// 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" };
|
||||
}
|
||||
if (fromCli?.forced) return forcedOutcome(fromCli.enabled);
|
||||
|
||||
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" };
|
||||
if (override === undefined) {
|
||||
if (isOptedOut()) return { enabled: false, reason: "telemetry_opt_out" };
|
||||
if (fromCli !== undefined) return cohortOutcome(fromCli.enabled);
|
||||
}
|
||||
|
||||
return evaluateCanary({
|
||||
|
||||
Reference in New Issue
Block a user