mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-11 06:30:03 +00:00
fix(cli,core): refresh telemetry posture at the render boundary
R6/R7 blockers. An already-open Studio kept emitting server-side render telemetry after another process disabled CLI telemetry. refreshTelemetryPosture() only ran while serving a fresh SPA document and on /api/telemetry-identity, which Studio has no consumer for, so the render POST and its async outcome used the posture cached when the preview server booted. It now refreshes at the render boundary and again immediately before the completion/error event, so an opt-out during a long render is honoured. The identity tests were passing vacuously: their mocks omitted readConfigFresh and resetTelemetryPostureCache, and the resulting missing-export error was swallowed by the refresh's own catch. Mocked properly, plus the enabled -> external disable -> next response transition and the suppression path at the layer that drops the event. A full reset also did not persist its new lineage in a long-lived process: syncInstallState returned early on a process-lifetime memo even after ~/.hyperframes was deleted, so install-state was never recreated and the next config-only re-mint rolled a third seed instead of inheriting the second. The memo is now revalidated against the file. Also drops a stale reference to assertNoOverdueCanaries and stops the workflow and docs claiming the sunset job routes anything to the owner — it names them in the run log and notifies nobody. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
cad6b394f4
commit
3f8dca165d
@@ -2,9 +2,12 @@
|
|||||||
#
|
#
|
||||||
# Deliberately NOT a PR gate. The check reads the current date, so as a PR gate
|
# Deliberately NOT a PR gate. The check reads the current date, so as a PR gate
|
||||||
# it would fail builds for authors who touched nothing related, on a calendar
|
# it would fail builds for authors who touched nothing related, on a calendar
|
||||||
# date, with no fix available to them. On a schedule the failure lands on the
|
# date, with no fix available to them. On a schedule the failure stands on its
|
||||||
# rollout's owner instead, which is who can actually ramp it to 100 and delete
|
# own instead of blocking an unrelated author.
|
||||||
# the guard.
|
#
|
||||||
|
# The job names the overdue canary and its owner in the run log; it does not
|
||||||
|
# notify anyone. Routing that to the owner automatically (an issue, a ping)
|
||||||
|
# is worth doing and is not done here.
|
||||||
name: Canary sunset
|
name: Canary sunset
|
||||||
|
|
||||||
permissions:
|
permissions:
|
||||||
|
|||||||
@@ -56,7 +56,8 @@ call site.
|
|||||||
**4. Delete it** once it is at 100 and holding — both the registry entry and
|
**4. Delete it** once it is at 100 and holding — both the registry entry and
|
||||||
the branch it guarded. `sunsetAfter` exists to force this: the scheduled
|
the branch it guarded. `sunsetAfter` exists to force this: the scheduled
|
||||||
**Canary sunset** workflow runs weekly and fails once the date passes, naming
|
**Canary sunset** workflow runs weekly and fails once the date passes, naming
|
||||||
the rollout and its owner.
|
the overdue rollout and its owner in the run log. It does not notify anyone —
|
||||||
|
watch the workflow if you own a canary.
|
||||||
|
|
||||||
It is a scheduled job rather than a PR check on purpose. A current-date
|
It is a scheduled job rather than a PR check on purpose. A current-date
|
||||||
assertion in the unit suite would redden builds for authors who changed
|
assertion in the unit suite would redden builds for authors who changed
|
||||||
|
|||||||
@@ -387,6 +387,10 @@ export function createStudioServer(options: StudioServerOptions): StudioServer {
|
|||||||
rendersDir: () => join(projectDir, "renders"),
|
rendersDir: () => join(projectDir, "renders"),
|
||||||
|
|
||||||
startRender(opts): RenderJobState {
|
startRender(opts): RenderJobState {
|
||||||
|
// The render POST is a request boundary like any other. Without this an
|
||||||
|
// already-open Studio tab keeps rendering under the posture cached when
|
||||||
|
// the server booted.
|
||||||
|
refreshTelemetryPosture();
|
||||||
const abortController = new AbortController();
|
const abortController = new AbortController();
|
||||||
const state: RenderJobState = {
|
const state: RenderJobState = {
|
||||||
id: opts.jobId,
|
id: opts.jobId,
|
||||||
@@ -466,6 +470,12 @@ export function createStudioServer(options: StudioServerOptions): StudioServer {
|
|||||||
metaPath,
|
metaPath,
|
||||||
JSON.stringify({ status: "complete", durationMs: Date.now() - startTime }),
|
JSON.stringify({ status: "complete", durationMs: Date.now() - startTime }),
|
||||||
);
|
);
|
||||||
|
// Refreshed HERE, not just at render start: a render can run for
|
||||||
|
// minutes, and `hyperframes telemetry disable` during one must be
|
||||||
|
// honoured by the event that reports it. Studio never polls
|
||||||
|
// /api/telemetry-identity, so this process would otherwise keep its
|
||||||
|
// startup-cached posture for the life of the preview server.
|
||||||
|
refreshTelemetryPosture();
|
||||||
emitStudioRenderComplete(opts, Date.now() - startTime, job.perfSummary);
|
emitStudioRenderComplete(opts, Date.now() - startTime, job.perfSummary);
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
if (abortController.signal.aborted) {
|
if (abortController.signal.aborted) {
|
||||||
@@ -475,6 +485,7 @@ export function createStudioServer(options: StudioServerOptions): StudioServer {
|
|||||||
state.status = "failed";
|
state.status = "failed";
|
||||||
state.error = err instanceof Error ? err.message : String(err);
|
state.error = err instanceof Error ? err.message : String(err);
|
||||||
// fallow-ignore-next-line code-duplication
|
// fallow-ignore-next-line code-duplication
|
||||||
|
refreshTelemetryPosture();
|
||||||
emitStudioRenderError(opts, Date.now() - startTime, state.stage, err, renderJob);
|
emitStudioRenderError(opts, Date.now() - startTime, state.stage, err, renderJob);
|
||||||
try {
|
try {
|
||||||
const metaPath = opts.outputPath.replace(/\.(mp4|webm|mov)$/, ".meta.json");
|
const metaPath = opts.outputPath.replace(/\.(mp4|webm|mov)$/, ".meta.json");
|
||||||
|
|||||||
@@ -11,11 +11,20 @@ const readConfig = vi.fn();
|
|||||||
// don't move every time a canary is added, ramped, or retired.
|
// don't move every time a canary is added, ramped, or retired.
|
||||||
const canaryDecisions = vi.fn<() => Record<string, { enabled: boolean; forced: boolean }>>();
|
const canaryDecisions = vi.fn<() => Record<string, { enabled: boolean; forced: boolean }>>();
|
||||||
|
|
||||||
|
// Every export the module under test imports must be mocked. Omitting
|
||||||
|
// `resetTelemetryPostureCache` / `readConfigFresh` made `refreshTelemetryPosture`
|
||||||
|
// throw a missing-export error that its own catch swallowed, so every
|
||||||
|
// assertion below ran against a refresh that silently did nothing.
|
||||||
|
const resetPostureCache = vi.fn();
|
||||||
|
const readConfigFresh = vi.fn();
|
||||||
|
|
||||||
vi.mock("../telemetry/client.js", () => ({
|
vi.mock("../telemetry/client.js", () => ({
|
||||||
shouldTrack: (...args: unknown[]) => shouldTrack(...args),
|
shouldTrack: (...args: unknown[]) => shouldTrack(...args),
|
||||||
|
resetTelemetryPostureCache: () => resetPostureCache(),
|
||||||
}));
|
}));
|
||||||
vi.mock("../telemetry/config.js", () => ({
|
vi.mock("../telemetry/config.js", () => ({
|
||||||
readConfig: (...args: unknown[]) => readConfig(...args),
|
readConfig: (...args: unknown[]) => readConfig(...args),
|
||||||
|
readConfigFresh: () => readConfigFresh(),
|
||||||
}));
|
}));
|
||||||
vi.mock("../telemetry/canary.js", () => ({
|
vi.mock("../telemetry/canary.js", () => ({
|
||||||
canaryDecisionsForStudio: () => canaryDecisions(),
|
canaryDecisionsForStudio: () => canaryDecisions(),
|
||||||
@@ -27,6 +36,7 @@ const {
|
|||||||
buildStudioHeadScripts,
|
buildStudioHeadScripts,
|
||||||
isLoopbackHost,
|
isLoopbackHost,
|
||||||
buildStudioHeadScriptsForHost,
|
buildStudioHeadScriptsForHost,
|
||||||
|
refreshTelemetryPosture,
|
||||||
identityAllowed,
|
identityAllowed,
|
||||||
} = await import("./telemetryIdentity.js");
|
} = await import("./telemetryIdentity.js");
|
||||||
|
|
||||||
@@ -344,3 +354,42 @@ describe("identityAllowed — loopback-bound vs explicitly LAN-bound", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// A long-lived preview server: the posture it cached at boot must not outlive
|
||||||
|
// an opt-out run in another terminal. Studio has no poller for
|
||||||
|
// /api/telemetry-identity, so the refresh has to happen on the paths that
|
||||||
|
// actually run — the SPA document and the render boundary.
|
||||||
|
describe("cross-process opt-out refresh", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
resetPostureCache.mockClear();
|
||||||
|
readConfigFresh.mockClear();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("actually invalidates both caches — the mocks used to swallow this", () => {
|
||||||
|
refreshTelemetryPosture();
|
||||||
|
expect(readConfigFresh).toHaveBeenCalledTimes(1);
|
||||||
|
expect(resetPostureCache).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("refreshes before building a head script", () => {
|
||||||
|
shouldTrack.mockReturnValue(true);
|
||||||
|
readConfig.mockReturnValue({ anonymousId: "id-1", bucketSeed: "seed-1" });
|
||||||
|
canaryDecisions.mockReturnValue({});
|
||||||
|
buildStudioHeadScriptsForHost("", "localhost:3000");
|
||||||
|
expect(resetPostureCache).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("stops publishing identity once another process disables telemetry", () => {
|
||||||
|
canaryDecisions.mockReturnValue({});
|
||||||
|
readConfig.mockReturnValue({ anonymousId: "id-1", bucketSeed: "seed-1" });
|
||||||
|
|
||||||
|
shouldTrack.mockReturnValue(true);
|
||||||
|
expect(buildStudioHeadScriptsForHost("", "localhost:3000")).toContain("__HF_CLI_DISTINCT_ID");
|
||||||
|
|
||||||
|
// `hyperframes telemetry disable` in another terminal.
|
||||||
|
shouldTrack.mockReturnValue(false);
|
||||||
|
const after = buildStudioHeadScriptsForHost("", "localhost:3000");
|
||||||
|
expect(after).not.toContain("__HF_CLI_DISTINCT_ID");
|
||||||
|
expect(after).not.toContain("__HF_CLI_BUCKET_SEED");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -0,0 +1,72 @@
|
|||||||
|
import { describe, expect, it, vi, beforeEach } from "vitest";
|
||||||
|
|
||||||
|
// The suppression half of the cross-process opt-out. `hyperframes preview` is
|
||||||
|
// long-lived, and Studio has no poller for /api/telemetry-identity, so a
|
||||||
|
// render that finishes AFTER `hyperframes telemetry disable` ran in another
|
||||||
|
// terminal used to report its outcome anyway: shouldTrack() had cached `true`
|
||||||
|
// at boot and nothing ever asked again.
|
||||||
|
//
|
||||||
|
// This pins the mechanism the render-outcome path depends on — refresh, then
|
||||||
|
// emit — at the layer where the event is actually dropped.
|
||||||
|
|
||||||
|
vi.stubEnv("HYPERFRAMES_NO_TELEMETRY", "");
|
||||||
|
vi.stubEnv("DO_NOT_TRACK", "");
|
||||||
|
|
||||||
|
const configState = { telemetryEnabled: true };
|
||||||
|
vi.mock("./config.js", () => ({
|
||||||
|
readConfig: () => ({ anonymousId: "anon-1", telemetryEnabled: configState.telemetryEnabled }),
|
||||||
|
writeConfig: () => {},
|
||||||
|
}));
|
||||||
|
vi.mock("../utils/env.js", () => ({ isDevMode: () => false }));
|
||||||
|
vi.mock("./canary.js", () => ({ canaryEventProperties: () => ({}) }));
|
||||||
|
|
||||||
|
const enqueue = vi.fn();
|
||||||
|
vi.mock("./transport.js", () => ({
|
||||||
|
enqueue: (...args: unknown[]) => enqueue(...args),
|
||||||
|
flush: vi.fn(),
|
||||||
|
flushSync: vi.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
const { trackEvent, resetTelemetryPostureCache, shouldTrack } = await import("./client.js");
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
configState.telemetryEnabled = true;
|
||||||
|
enqueue.mockClear();
|
||||||
|
resetTelemetryPostureCache();
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("telemetry posture refresh", () => {
|
||||||
|
it("stops emitting once another process disables telemetry", () => {
|
||||||
|
trackEvent("render_complete", {});
|
||||||
|
expect(enqueue).toHaveBeenCalledTimes(1);
|
||||||
|
|
||||||
|
// `hyperframes telemetry disable` elsewhere, mid-render.
|
||||||
|
configState.telemetryEnabled = false;
|
||||||
|
resetTelemetryPostureCache();
|
||||||
|
|
||||||
|
trackEvent("render_complete", {});
|
||||||
|
expect(enqueue, "outcome emitted after the user opted out").toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
// The memo is load-bearing for a CLI command — one process, one answer, asked
|
||||||
|
// once per event. Dropping it entirely would be a per-event config read.
|
||||||
|
it("still caches within a posture, so it is not a per-event disk read", () => {
|
||||||
|
expect(shouldTrack()).toBe(true);
|
||||||
|
configState.telemetryEnabled = false;
|
||||||
|
expect(shouldTrack(), "changed without an explicit refresh").toBe(true);
|
||||||
|
resetTelemetryPostureCache();
|
||||||
|
expect(shouldTrack()).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("re-enables after the user opts back in", () => {
|
||||||
|
configState.telemetryEnabled = false;
|
||||||
|
resetTelemetryPostureCache();
|
||||||
|
trackEvent("render_complete", {});
|
||||||
|
expect(enqueue).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
configState.telemetryEnabled = true;
|
||||||
|
resetTelemetryPostureCache();
|
||||||
|
trackEvent("render_complete", {});
|
||||||
|
expect(enqueue).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -276,6 +276,27 @@ describe("install-state rollover (breaker survives a config re-mint)", () => {
|
|||||||
expect(readConfigFresh().predecessorFound).toBeUndefined();
|
expect(readConfigFresh().predecessorFound).toBeUndefined();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// A full reset in a LONG-LIVED process. The memo that says "this process
|
||||||
|
// already mirrored the state file" stayed set after the file was deleted, so
|
||||||
|
// the mirror was never recreated: the freshly minted seed lived in
|
||||||
|
// config.json alone, and the NEXT config-only re-mint rolled a THIRD seed
|
||||||
|
// instead of inheriting the second. The old test stopped at the second mint
|
||||||
|
// and so missed the durability half entirely.
|
||||||
|
it("recreates install-state after a full wipe, so the new seed's lineage is durable", () => {
|
||||||
|
const first = readConfig().bucketSeed;
|
||||||
|
fsState.files.delete(CONFIG_PATH);
|
||||||
|
fsState.files.delete(STATE_PATH);
|
||||||
|
|
||||||
|
const second = readConfigFresh().bucketSeed;
|
||||||
|
expect(second, "a full reset must mint a new cohort").not.toBe(first);
|
||||||
|
expect(fsState.files.has(STATE_PATH), "state file must be recreated").toBe(true);
|
||||||
|
expect(stateFile()["bucketSeed"]).toBe(second);
|
||||||
|
|
||||||
|
// Config-only re-mint: the seed must now be inherited, not rolled again.
|
||||||
|
fsState.files.delete(CONFIG_PATH);
|
||||||
|
expect(readConfigFresh().bucketSeed, "lineage lost after reset").toBe(second);
|
||||||
|
});
|
||||||
|
|
||||||
// The move: state used to live in ~/.local/state/hyperframes/ so it would
|
// The move: state used to live in ~/.local/state/hyperframes/ so it would
|
||||||
// survive `rm -rf ~/.hyperframes`. Review rejected persisting state outside
|
// survive `rm -rf ~/.hyperframes`. Review rejected persisting state outside
|
||||||
// the config dir to defeat the user's reset, so it now shares CONFIG_DIR.
|
// the config dir to defeat the user's reset, so it now shares CONFIG_DIR.
|
||||||
|
|||||||
@@ -317,6 +317,16 @@ function applyInstallState(config: HyperframesConfig, wantFired: boolean): void
|
|||||||
|
|
||||||
function syncInstallState(config: HyperframesConfig): boolean {
|
function syncInstallState(config: HyperframesConfig): boolean {
|
||||||
const wantFired = config.deParallelRouterTrialFired === true;
|
const wantFired = config.deParallelRouterTrialFired === true;
|
||||||
|
// The memo says "this process already wrote the state file". That is only
|
||||||
|
// true while the file is still there. `rm -rf ~/.hyperframes` under a
|
||||||
|
// long-lived preview left the memo set, so the mirror was never recreated:
|
||||||
|
// the freshly minted seed lived in config.json alone, and the NEXT
|
||||||
|
// config-only re-mint rolled a third seed instead of inheriting the second.
|
||||||
|
// One existsSync on a path we are about to write anyway.
|
||||||
|
if (stateMarkerSynced && !existsSync(STATE_FILE)) {
|
||||||
|
stateMarkerSynced = false;
|
||||||
|
stateFiredSynced = false;
|
||||||
|
}
|
||||||
if (stateMarkerSynced && (stateFiredSynced || !wantFired)) return true;
|
if (stateMarkerSynced && (stateFiredSynced || !wantFired)) return true;
|
||||||
try {
|
try {
|
||||||
applyInstallState(config, wantFired);
|
applyInstallState(config, wantFired);
|
||||||
|
|||||||
@@ -38,8 +38,9 @@ export interface CanaryDefinition {
|
|||||||
/**
|
/**
|
||||||
* ISO date after which this canary is overdue for removal. A canary that
|
* 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
|
* outlives its rollout is a permanent fork of the product with none of the
|
||||||
* review a permanent fork would have received. `assertNoOverdueCanaries`
|
* review a permanent fork would have received. The scheduled `Canary sunset`
|
||||||
* turns the date into a failing test rather than a good intention.
|
* workflow runs `scripts/check-canary-sunset.ts` weekly and fails once the
|
||||||
|
* date passes, so this is an enforced deadline rather than a good intention.
|
||||||
*/
|
*/
|
||||||
sunsetAfter: string;
|
sunsetAfter: string;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user