Merge pull request #2874 from heygen-com/feat/breaker-carryover

feat(cli): roll circuit-breaker state over across config wipes

Author: vanceingalls

packages/cli/src/commands/telemetry.test.ts

@@ -27,6 +27,7 @@ async function loadTelemetryCommand(options?: {
   vi.resetModules();
   vi.doMock("../telemetry/config.js", () => ({
     CONFIG_PATH: "/test/.hyperframes/config.json",
+    STATE_PATH: "/test/.local/state/hyperframes/install-state.json",
     readConfig: () => {
       throw new Error("telemetry commands must bypass stale cached config");
     },

packages/cli/src/commands/telemetry.ts

@@ -1,5 +1,10 @@
 import { defineCommand } from "citty";
-import { writeConfigWithResult, readConfigFresh, CONFIG_PATH } from "../telemetry/config.js";
+import {
+  writeConfigWithResult,
+  readConfigFresh,
+  CONFIG_PATH,
+  STATE_PATH,
+} from "../telemetry/config.js";
 import { effectiveTelemetryStatus, type TelemetryStatusSource } from "../telemetry/policy.js";
 import { c } from "../ui/colors.js";
 import { failCommand } from "../utils/commandResult.js";
@@ -56,6 +61,9 @@ function runStatus(): void {
   console.log(`  ${c.dim("Status:")}     ${status}`);
   console.log(`  ${c.dim("Source:")}     ${effective.source}`);
   console.log(`  ${c.dim("Config:")}     ${c.accent(CONFIG_PATH)}`);
+  // Machine-local safety state (no identity): survives a config wipe so a
+  // tripped experiment circuit breaker stays tripped. Listed for transparency.
+  console.log(`  ${c.dim("State:")}      ${c.accent(STATE_PATH)}`);
   console.log(`  ${c.dim("Tracked commands:")} ${c.bold(String(config.commandCount))}`);
   console.log();
   console.log(`  ${c.dim("Disable:")}    ${c.accent("hyperframes telemetry disable")}`);

packages/cli/src/telemetry/client.ts

@@ -73,6 +73,12 @@ export function trackEvent(
       // New-agent discovery signals — populated only when agent_runtime is null.
       agent_hint: sys.agent_hint ?? undefined,
       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
+      // we already knew. Absent (not false) when the config predates the
+      // marker. Resolved after the shouldTrack guard.
+      install_predecessor_found: readConfig().predecessorFound,
       agent_env_hints: sys.agent_env_hints ?? undefined,
     },
     distinctId,

packages/cli/src/telemetry/config.test.ts

@@ -129,3 +129,120 @@ describe("config.ts — readConfig / readConfigFresh / writeConfig (real module,
     });
   });
 });
+
+describe("install-state rollover (breaker survives 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"));
+  });
+
+  /** Simulate the identity reset this feature exists for. */
+  function wipeConfig(): void {
+    fsState.files.delete(CONFIG_PATH);
+  }
+
+  function stateFile(): Record<string, unknown> {
+    const raw = fsState.files.get(STATE_PATH);
+    expect(raw, "install-state file should exist").toBeDefined();
+    return JSON.parse(raw as string) as Record<string, unknown>;
+  }
+
+  it("a truly fresh install writes the marker and records predecessorFound: false", () => {
+    const config = readConfig();
+    expect(config.predecessorFound).toBe(false);
+    expect(stateFile()["markerAt"]).toEqual(expect.any(String));
+  });
+
+  it("a re-mint after a config wipe finds the marker: predecessorFound is true, id is fresh", () => {
+    const first = readConfig();
+    wipeConfig();
+    const second = readConfigFresh();
+    expect(second.predecessorFound).toBe(true);
+    // The rollover carries safety state, never identity.
+    expect(second.anonymousId).not.toBe(first.anonymousId);
+  });
+
+  it("a tripped breaker survives a config wipe — the whole point", () => {
+    const config = readConfig();
+    config.deParallelRouterTrialFired = true;
+    writeConfig(config);
+    wipeConfig();
+    const reborn = readConfigFresh();
+    expect(reborn.deParallelRouterTrialFired).toBe(true);
+  });
+
+  it("an untripped breaker does NOT get invented by the rollover", () => {
+    readConfig();
+    wipeConfig();
+    expect(readConfigFresh().deParallelRouterTrialFired).toBeUndefined();
+  });
+
+  it("a tripped breaker survives config CORRUPTION via the same path", () => {
+    const config = readConfig();
+    config.deParallelRouterTrialFired = true;
+    writeConfig(config);
+    fsState.files.set(CONFIG_PATH, "{not valid json");
+    expect(readConfigFresh().deParallelRouterTrialFired).toBe(true);
+  });
+
+  it("the state file holds no identity — only the marker timestamp and breaker fact", () => {
+    const config = readConfig();
+    config.deParallelRouterTrialFired = true;
+    writeConfig(config);
+    expect(Object.keys(stateFile()).sort()).toEqual(["deParallelRouterTrialFired", "markerAt"]);
+    expect(JSON.stringify(stateFile())).not.toContain(config.anonymousId);
+  });
+
+  it("marker timestamp is written once, not refreshed by later config writes", () => {
+    const config = readConfig();
+    const minted = stateFile()["markerAt"];
+    config.commandCount = 42;
+    config.deParallelRouterTrialFired = true;
+    writeConfig(config);
+    expect(stateFile()["markerAt"]).toBe(minted);
+  });
+
+  it("a corrupted state file reads as absent rather than breaking the mint", () => {
+    fsState.files.set(STATE_PATH, "{not valid json");
+    const config = readConfig();
+    expect(config.predecessorFound).toBe(false);
+    expect(config.anonymousId).toBeTruthy();
+    // And the corrupt file was replaced with a valid marker by the mint's write.
+    expect(stateFile()["markerAt"]).toEqual(expect.any(String));
+  });
+
+  it("a state-file write failure never breaks the config write", async () => {
+    const fs = await import("node:fs");
+    const config = readConfig(); // marker already written by the mint
+    fsState.files.delete(STATE_PATH); // force a re-sync attempt...
+    const { __resetInstallStateSyncForTests } = await import("./config.js");
+    __resetInstallStateSyncForTests();
+    let calls = 0;
+    vi.mocked(fs.writeFileSync).mockImplementation((path, content) => {
+      calls++;
+      if (String(path).startsWith(STATE_PATH)) throw new Error("EACCES");
+      fsState.files.set(String(path), String(content));
+    });
+    config.commandCount = 1;
+    expect(writeConfig(config)).toBe(true); // ...that fails, swallowed
+    expect(calls).toBeGreaterThan(1);
+    vi.mocked(fs.writeFileSync).mockImplementation((path, content) => {
+      fsState.files.set(String(path), String(content));
+    });
+  });
+
+  it("predecessorFound on an EXISTING config predating the field reads as undefined, not false", () => {
+    const base = readConfig();
+    const { predecessorFound: _dropped, ...legacy } = base;
+    fsState.files.set(CONFIG_PATH, JSON.stringify(legacy));
+    expect(readConfigFresh().predecessorFound).toBeUndefined();
+  });
+});

packages/cli/src/telemetry/config.ts

@@ -11,6 +11,128 @@ import { normalizeErrorMessage } from "../utils/errorMessage.js";
 const CONFIG_DIR = join(homedir(), ".hyperframes");
 const CONFIG_FILE = join(CONFIG_DIR, "config.json");
 
+// ---------------------------------------------------------------------------
+// Install-state file: ~/.local/state/hyperframes/install-state.json
+//
+// A second, deliberately separate location from CONFIG_DIR, so it survives
+// the most common identity reset — deleting or reinstalling ~/.hyperframes.
+// It exists to carry exactly two facts across that reset, and nothing else:
+//
+//   1. `markerAt` — "a hyperframes install existed on this machine". Written
+//      unconditionally, so the fraction of fresh installs that find it is a
+//      direct measurement of recoverable id churn (config wiped, machine
+//      persisted) vs unrecoverable (fresh machine/container/new user).
+//   2. `deParallelRouterTrialFired` — the DE parallel-router circuit
+//      breaker's tripped state. Without this, a config wipe re-enrols the
+//      install into an experimental path that already FAILED on this exact
+//      machine; the breaker's whole point is that a real failure turns the
+//      trial off for good.
+//
+// It intentionally holds NO identity: no anonymousId, no counters, nothing
+// that could link the old install to the new one. A user who wipes their
+// config gets a fresh id unconditionally — this file only stops the wipe
+// from also discarding a safety fact about the machine.
+// ---------------------------------------------------------------------------
+
+const STATE_DIR = join(homedir(), ".local", "state", "hyperframes");
+const STATE_FILE = join(STATE_DIR, "install-state.json");
+
+interface InstallState {
+  /** ISO timestamp of when the marker was first written. */
+  markerAt: string;
+  /** Rolled-over circuit-breaker state — see HyperframesConfig's field. */
+  deParallelRouterTrialFired?: boolean;
+}
+
+/** Read the install-state file; any parse/shape failure reads as absent. */
+function readInstallState(): InstallState | null {
+  try {
+    if (!existsSync(STATE_FILE)) return null;
+    const parsed = JSON.parse(readFileSync(STATE_FILE, "utf-8")) as Partial<InstallState>;
+    if (typeof parsed.markerAt !== "string") return null;
+    return {
+      markerAt: parsed.markerAt,
+      deParallelRouterTrialFired: parsed.deParallelRouterTrialFired === true ? true : undefined,
+    };
+  } catch {
+    return null;
+  }
+}
+
+// Sync bookkeeping, so the existsSync+read doesn't run on every writeConfig:
+// `stateMarkerSynced` = the marker is known present; `stateFiredSynced` = the
+// state file is known to already carry fired=true.
+let stateMarkerSynced = false;
+let stateFiredSynced = false;
+
+/** Test-only: reset the sync memo (module state leaks across vitest cases). */
+export function __resetInstallStateSyncForTests(): void {
+  stateMarkerSynced = false;
+  stateFiredSynced = false;
+}
+
+/**
+ * Atomic for the same reason writeConfig is: a torn read must never exist,
+ * since a corrupted state file silently reads as absent.
+ */
+function writeInstallState(next: InstallState): void {
+  mkdirSync(STATE_DIR, { recursive: true, mode: 0o700 });
+  const tmpFile = `${STATE_FILE}.${process.pid}.tmp`;
+  writeFileSync(tmpFile, JSON.stringify(next, null, 2) + "\n", { mode: 0o600 });
+  renameSync(tmpFile, STATE_FILE);
+}
+
+/**
+ * Bring the install-state file up to date with this config write: ensure the
+ * marker exists, and mirror a tripped breaker. Called from `writeConfig` so
+ * no breaker write site can forget it. Never throws — same contract as the
+ * rest of this file, telemetry must not break the CLI.
+ */
+/** 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 {
+    markerAt: state?.markerAt ?? new Date().toISOString(),
+    deParallelRouterTrialFired: wantFired || undefined,
+  };
+}
+
+function syncInstallState(config: HyperframesConfig): void {
+  const wantFired = config.deParallelRouterTrialFired === true;
+  if (stateMarkerSynced && (stateFiredSynced || !wantFired)) return;
+  try {
+    const state = readInstallState();
+    const next = nextInstallState(state, wantFired);
+    if (next !== null) writeInstallState(next);
+    stateMarkerSynced = true;
+    stateFiredSynced = wantFired || state?.deParallelRouterTrialFired === true;
+  } catch {
+    // Leave the memo unset so a later write retries.
+  }
+}
+
+/**
+ * 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();
+  return {
+    ...DEFAULT_CONFIG,
+    anonymousId: randomUUID(),
+    predecessorFound: state !== null,
+    // The rollover itself: a breaker tripped by a previous install on this
+    // machine stays tripped for the new one.
+    deParallelRouterTrialFired: state?.deParallelRouterTrialFired === true ? true : undefined,
+  };
+}
+
 export interface HyperframesConfig {
   /** Whether anonymous telemetry is enabled (default: true in production) */
   telemetryEnabled: boolean;
@@ -86,6 +208,14 @@ export interface HyperframesConfig {
    * `DE_PARALLEL_ROUTER_TRIAL_MAX_RENDERS` in `render.ts`.
    */
   deParallelRouterTrialRenderCount?: number;
+  /**
+   * 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
+   * measurable directly. `undefined` on configs minted before this field
+   * existed — a different fact from `false` (minted fresh, no predecessor).
+   */
+  predecessorFound?: boolean;
   /**
    * Ring of the last few local renders (newest last). `hyperframes feedback`
    * attaches these ids — which are the `render_job_id` /
@@ -140,7 +270,7 @@ export function readConfig(): HyperframesConfig {
   if (cachedConfig) return { ...cachedConfig };
 
   if (!existsSync(CONFIG_FILE)) {
-    const config = { ...DEFAULT_CONFIG, anonymousId: randomUUID() };
+    const config = mintConfig();
     writeConfig(config);
     return config;
   }
@@ -176,6 +306,8 @@ export function readConfig(): HyperframesConfig {
         typeof parsed.deParallelRouterTrialRenderCount === "number"
           ? parsed.deParallelRouterTrialRenderCount
           : undefined,
+      predecessorFound:
+        typeof parsed.predecessorFound === "boolean" ? parsed.predecessorFound : undefined,
       recentRenders: Array.isArray(parsed.recentRenders)
         ? parsed.recentRenders
             .filter(
@@ -195,14 +327,10 @@ export function readConfig(): HyperframesConfig {
   } catch {
     // A missing file is handled above. Any failure here means an existing
     // preference could not be read safely (corrupt JSON, permissions, I/O).
-    // Preserve the historical recovery behavior for the rest of the config,
-    // but fail closed for the privacy control: recovery must never silently
-    // turn telemetry back on.
-    const config = {
-      ...DEFAULT_CONFIG,
-      telemetryEnabled: false,
-      anonymousId: randomUUID(),
-    };
+    // Recover through the same mint path as a missing file — so a tripped
+    // breaker survives config corruption too — but fail closed for the
+    // privacy control: recovery must never silently turn telemetry back on.
+    const config = { ...mintConfig(), telemetryEnabled: false };
     writeConfig(config);
     return config;
   }
@@ -254,6 +382,9 @@ export function writeConfigWithResult(config: HyperframesConfig): ConfigWriteRes
     writeFileSync(tmpFile, JSON.stringify(config, null, 2) + "\n", { mode: 0o600 });
     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 };
   } catch (error) {
     // Non-fatal — telemetry should never break the CLI
@@ -273,3 +404,6 @@ export function incrementCommandCount(): number {
 
 /** Expose the config directory path for the telemetry command output */
 export const CONFIG_PATH = CONFIG_FILE;
+
+/** Expose the install-state path for the telemetry command output. */
+export const STATE_PATH = STATE_FILE;