From 769dc702c1d62b86f1f5b2a05d76af39fc1be6b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Mon, 6 Jul 2026 16:59:47 -0400 Subject: [PATCH] feat(cli): emit sign-in lifecycle telemetry (#2000) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(cli): emit sign-in lifecycle telemetry The CLI tracks command and render lifecycles but emits nothing for `auth login`, so sign-in outcomes are invisible on the observability dashboards — a completed sign-in, an abandoned browser flow, and a rejected key all look identical (absent). This leaves a blind spot in the same funnel the render events already cover. Add three events mirroring the existing `trackX` pattern: - auth_login_started (method: oauth | api_key) - auth_login_completed (method) - auth_login_failed (method, reason) `reason` is a fixed low-cardinality enum (flow_error / no_credential / rejected / invalid_input). No token, key, identity, email, or free text is ever attached — consistent with the existing anonymous telemetry and the `telemetry disable` opt-out. Wired into both the OAuth and --api-key paths in `auth login`, with unit coverage for the new events. * fix(cli): close sign-in telemetry funnel dropout gaps Follow-up so `started` reconciles to `completed + failed` on the common abandonment paths, which the first cut missed: - Interactive prompt cancel (Ctrl-C) now surfaces as a throw that the single catch in the api-key path records as `aborted`, instead of a bare exit with no event. - A stdin read that times out in non-TTY `--api-key` mode now records `aborted` before the error propagates, rather than exiting silently. - OAuth split: a timed-out browser callback (user closed the tab) is tagged `flow_timeout`, separated from real `flow_error` (IdP/network), since the walk-away timeout is the dominant non-error dropout. Also pre-plumb an optional `distinctId` on the three trackers, mirroring trackRenderComplete/trackRenderError. Unused today; it lets a later identity-level attribution be a one-line callsite change rather than a signature sweep. Coverage added for the new reasons and forwarding. --- packages/cli/src/commands/auth/login.ts | 45 +++++++++++++-- packages/cli/src/telemetry/events.test.ts | 68 +++++++++++++++++++++++ packages/cli/src/telemetry/events.ts | 39 +++++++++++++ packages/cli/src/telemetry/index.ts | 3 + 4 files changed, 151 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/commands/auth/login.ts b/packages/cli/src/commands/auth/login.ts index 6058857a1..5c68be654 100644 --- a/packages/cli/src/commands/auth/login.ts +++ b/packages/cli/src/commands/auth/login.ts @@ -78,10 +78,19 @@ export default defineCommand({ async function runOAuthLogin(): Promise { assertOAuthConfiguredOrExit(); + const { trackAuthLoginStarted, trackAuthLoginFailed } = await import("../../telemetry/index.js"); + trackAuthLoginStarted("oauth"); + try { await startAuthorizationCodeFlow(); } catch (err) { - console.error(c.error(`Sign-in failed: ${(err as Error).message}`)); + const message = (err as Error).message ?? ""; + // The loopback server rejects with "OAuth callback timed out after …" when + // the user never completes the browser step (closed the tab / walked away). + // That is the dominant non-error dropout, so split it from real failures + // (IdP misconfig, network) instead of lumping everything as flow_error. + trackAuthLoginFailed("oauth", /timed out/i.test(message) ? "flow_timeout" : "flow_error"); + console.error(c.error(`Sign-in failed: ${message}`)); process.exit(1); } @@ -90,11 +99,18 @@ async function runOAuthLogin(): Promise { // fallow-ignore-next-line complexity async function reportIdentity(): Promise { + const { trackAuthLoginCompleted, trackAuthLoginFailed } = + await import("../../telemetry/index.js"); const credential = await tryResolveCredential(); if (!credential) { + trackAuthLoginFailed("oauth", "no_credential"); console.error(c.warn("Sign-in completed but no credential was persisted.")); process.exit(1); } + // A resolvable credential IS the success signal: the tokens are on disk and + // usable. The `/v3/users/me` probe below only fetches a display name, so its + // outcome is cosmetic and does not gate completion. + trackAuthLoginCompleted("oauth"); // Wire the refresh hook here too — a freshly-minted token shouldn't // need it, but a fast IdP-side rotation (or a misconfigured short // TTL) shouldn't punish the user with a hard failure when the @@ -163,8 +179,24 @@ async function clearUserInfoBestEffort(): Promise { // fallow-ignore-next-line complexity async function runApiKeyLogin(inlineKey: string): Promise { - const key = await collectApiKey(inlineKey); + const { trackAuthLoginStarted, trackAuthLoginCompleted, trackAuthLoginFailed } = + await import("../../telemetry/index.js"); + trackAuthLoginStarted("api_key"); + + // collectApiKey throws when the user cancels the interactive prompt (Ctrl-C) + // or when no key arrives on stdin before the timeout — both are "user walked + // away", the abandonment signal we most want. Record it before the error + // propagates so `started` still reconciles to `completed + failed`. + let key: string; + try { + key = await collectApiKey(inlineKey); + } catch (err) { + trackAuthLoginFailed("api_key", "aborted"); + console.error(c.error((err as Error).message || "Sign-in aborted.")); + process.exit(1); + } if (!key) { + trackAuthLoginFailed("api_key", "invalid_input"); console.error(c.error("No API key provided.")); process.exit(1); } @@ -172,10 +204,12 @@ async function runApiKeyLogin(inlineKey: string): Promise { // CR/LF in the value would smuggle headers when the key is sent // via `x-api-key`. The backend handles "wrong key" itself, but // header-injection has to be caught here. + trackAuthLoginFailed("api_key", "invalid_input"); console.error(c.error("API key must not contain newline or control characters.")); process.exit(1); } if (key.length < MIN_KEY_LENGTH) { + trackAuthLoginFailed("api_key", "invalid_input"); console.error(c.error(`API key looks too short (got ${key.length} chars).`)); process.exit(1); } @@ -186,9 +220,11 @@ async function runApiKeyLogin(inlineKey: string): Promise { const verifyOk = await verifyAndReport(key); if (!verifyOk) { + trackAuthLoginFailed("api_key", "rejected"); await rollback(previous); process.exit(1); } + trackAuthLoginCompleted("api_key"); } async function snapshotStore(): Promise { @@ -288,8 +324,9 @@ async function promptForKey(): Promise { }, }); if (clack.isCancel(value)) { - console.error("Aborted."); - process.exit(1); + // Throw rather than exit here so the single catch in runApiKeyLogin records + // the abandonment (auth_login_failed: aborted) and then exits. + throw new Error("Aborted."); } return value.trim(); } diff --git a/packages/cli/src/telemetry/events.test.ts b/packages/cli/src/telemetry/events.test.ts index 1a4aa0890..4a474ad19 100644 --- a/packages/cli/src/telemetry/events.test.ts +++ b/packages/cli/src/telemetry/events.test.ts @@ -14,6 +14,9 @@ const { trackFigmaImport, trackRenderFeedback, trackRenderPreflightRejected, + trackAuthLoginStarted, + trackAuthLoginCompleted, + trackAuthLoginFailed, } = await import("./events.js"); describe("render telemetry events", () => { @@ -226,3 +229,68 @@ describe("trackFigmaImport", () => { ); }); }); + +describe("auth login telemetry events", () => { + beforeEach(() => { + trackEvent.mockClear(); + }); + + it("emits auth_login_started tagged with the method", () => { + trackAuthLoginStarted("oauth"); + expect(trackEvent).toHaveBeenCalledWith("auth_login_started", { method: "oauth" }, undefined); + }); + + it("emits auth_login_completed tagged with the method", () => { + trackAuthLoginCompleted("api_key"); + expect(trackEvent).toHaveBeenCalledWith( + "auth_login_completed", + { method: "api_key" }, + undefined, + ); + }); + + it("emits auth_login_failed with the method and a low-cardinality reason", () => { + trackAuthLoginFailed("oauth", "flow_error"); + expect(trackEvent).toHaveBeenCalledWith( + "auth_login_failed", + { method: "oauth", reason: "flow_error" }, + undefined, + ); + }); + + it("distinguishes a timed-out browser flow from a real error", () => { + trackAuthLoginFailed("oauth", "flow_timeout"); + expect(trackEvent).toHaveBeenCalledWith( + "auth_login_failed", + { method: "oauth", reason: "flow_timeout" }, + undefined, + ); + }); + + it("records an aborted prompt / stdin timeout as its own reason", () => { + trackAuthLoginFailed("api_key", "aborted"); + expect(trackEvent).toHaveBeenCalledWith( + "auth_login_failed", + { method: "api_key", reason: "aborted" }, + undefined, + ); + }); + + it("carries only method + reason — never a key, token, or free text", () => { + trackAuthLoginFailed("api_key", "rejected"); + expect(trackEvent).toHaveBeenCalledWith( + "auth_login_failed", + { method: "api_key", reason: "rejected" }, + undefined, + ); + }); + + it("forwards an explicit distinctId to trackEvent for future user-level attribution", () => { + trackAuthLoginCompleted("oauth", "heygen-user-123"); + expect(trackEvent).toHaveBeenCalledWith( + "auth_login_completed", + { method: "oauth" }, + "heygen-user-123", + ); + }); +}); diff --git a/packages/cli/src/telemetry/events.ts b/packages/cli/src/telemetry/events.ts index ec4bf891d..6671460b1 100644 --- a/packages/cli/src/telemetry/events.ts +++ b/packages/cli/src/telemetry/events.ts @@ -308,6 +308,45 @@ export function trackBrowserInstall(): void { trackEvent("browser_install", {}); } +// Sign-in lifecycle. The CLI tracks command and render lifecycles but never +// authentication, so `auth login` outcomes are invisible on the observability +// dashboards — a completed sign-in, a browser flow the user abandoned, and a +// rejected key all look identical (i.e. absent). These three events close that +// gap so the sign-in funnel is measurable like the render funnel already is. +// `method` is "oauth" (the default browser PKCE flow) or "api_key". No token, +// key, identity, email, or free text is ever attached — only the method and a +// low-cardinality outcome/reason. +// +// The three trackers accept an optional `distinctId`, forwarded to trackEvent +// exactly like trackRenderComplete/trackRenderError already do. It is unused +// today (events attribute to the install's anonymousId), but pre-plumbing it +// makes attributing a completed sign-in to a resolved identity later a one-line +// change at the callsite rather than a signature sweep. +export type AuthLoginMethod = "oauth" | "api_key"; +export type AuthLoginFailureReason = + | "flow_error" // OAuth authorization/exchange threw a real error + | "flow_timeout" // OAuth callback wait elapsed (user closed the tab / walked away) + | "no_credential" // flow reported success but nothing was persisted + | "rejected" // backend rejected the supplied API key (401) + | "invalid_input" // key was empty, header-unsafe, or too short + | "aborted"; // prompt cancelled, or no key arrived on stdin before timeout + +export function trackAuthLoginStarted(method: AuthLoginMethod, distinctId?: string): void { + trackEvent("auth_login_started", { method }, distinctId); +} + +export function trackAuthLoginCompleted(method: AuthLoginMethod, distinctId?: string): void { + trackEvent("auth_login_completed", { method }, distinctId); +} + +export function trackAuthLoginFailed( + method: AuthLoginMethod, + reason: AuthLoginFailureReason, + distinctId?: string, +): void { + trackEvent("auth_login_failed", { method, reason }, distinctId); +} + // A render was rejected by the output-resolution/alpha/HDR pre-flight (P1-3) // before any browser/ffmpeg work. Counts the "caught early" saves on dashboard // 1783183, distinct from deep render failures. `kind` is the low-cardinality diff --git a/packages/cli/src/telemetry/index.ts b/packages/cli/src/telemetry/index.ts index 8da1cb110..854ee03ed 100644 --- a/packages/cli/src/telemetry/index.ts +++ b/packages/cli/src/telemetry/index.ts @@ -9,5 +9,8 @@ export { trackCliError, trackCommandResult, trackFigmaImport, + trackAuthLoginStarted, + trackAuthLoginCompleted, + trackAuthLoginFailed, } from "./events.js"; export { getSystemMeta, getShmSizeMb, getFreeDiskMb, bytesToMb } from "./system.js";