feat(cli): emit sign-in lifecycle telemetry (#2000)

* 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.
This commit is contained in:
Miguel Ángel
2026-07-06 16:59:47 -04:00
committed by GitHub
parent a4989dbe75
commit 769dc702c1
4 changed files with 151 additions and 4 deletions
+41 -4
View File
@@ -78,10 +78,19 @@ export default defineCommand({
async function runOAuthLogin(): Promise<void> {
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<void> {
// fallow-ignore-next-line complexity
async function reportIdentity(): Promise<void> {
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<void> {
// fallow-ignore-next-line complexity
async function runApiKeyLogin(inlineKey: string): Promise<void> {
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<void> {
// 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<void> {
const verifyOk = await verifyAndReport(key);
if (!verifyOk) {
trackAuthLoginFailed("api_key", "rejected");
await rollback(previous);
process.exit(1);
}
trackAuthLoginCompleted("api_key");
}
async function snapshotStore(): Promise<Credentials> {
@@ -288,8 +324,9 @@ async function promptForKey(): Promise<string> {
},
});
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();
}