diff --git a/packages/cli/src/commands/telemetry.test.ts b/packages/cli/src/commands/telemetry.test.ts index 93f39e7f15..3fe6150328 100644 --- a/packages/cli/src/commands/telemetry.test.ts +++ b/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"); }, diff --git a/packages/cli/src/commands/telemetry.ts b/packages/cli/src/commands/telemetry.ts index 8ffb4b119e..0b24e5ec14 100644 --- a/packages/cli/src/commands/telemetry.ts +++ b/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")}`); diff --git a/packages/cli/src/telemetry/client.ts b/packages/cli/src/telemetry/client.ts index 2e6f085a7d..24692f8679 100644 --- a/packages/cli/src/telemetry/client.ts +++ b/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, diff --git a/packages/cli/src/telemetry/config.test.ts b/packages/cli/src/telemetry/config.test.ts index d24a228f12..dfda8f3fdf 100644 --- a/packages/cli/src/telemetry/config.test.ts +++ b/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 { + const raw = fsState.files.get(STATE_PATH); + expect(raw, "install-state file should exist").toBeDefined(); + return JSON.parse(raw as string) as Record; + } + + 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(); + }); +}); diff --git a/packages/cli/src/telemetry/config.ts b/packages/cli/src/telemetry/config.ts index 48ded16c5a..7b53c6d8b5 100644 --- a/packages/cli/src/telemetry/config.ts +++ b/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; + 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;