Skip to content

Commit 33a7896

Browse files
committed
Report which provider rejected credentials on auth failure
The auth_failure event reused error_class, which everywhere else means the JS error constructor name. One column carrying two incompatible meanings cannot be analysed, and it made the documented error_class guarantee false: the value shipped was a local send-failure kind that never passed through the classifier.
1 parent 942de52 commit 33a7896

5 files changed

Lines changed: 68 additions & 6 deletions

File tree

docs/TELEMETRY.md

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ Each event carries a small set of properties:
2121
| `permission_prompt` | An approval prompt is answered (or abandoned) | `decision`, `permission_kind` |
2222
| `compaction` | The compactor actually folds turns away | `mode`, `duration_ms`, `turns_before`, `turns_after` |
2323
| `crash` | A fatal error reaches the process-level handler | `kind`, `error_class` |
24-
| `auth_failure` | A provider rejects the stored credentials | `error_class` |
24+
| `auth_failure` | A provider rejects the stored credentials | `auth_provider` |
2525

2626
`compaction` is deliberately silent on the runs where the compactor decides
2727
there is nothing to compact — an event that also fires on no-ops makes its own
@@ -62,7 +62,14 @@ all and `plugin_loaded` carries only `origin`, the discovery tier
6262

6363
`error_class` is bucketed the same way: only the error types defined by the
6464
language are reported by name, because an error subclass defined in
65-
application or plugin code is as author-chosen as any other string.
65+
application or plugin code is as author-chosen as any other string. It appears
66+
on `crash` and nowhere else, so the column means one thing everywhere it is
67+
recorded.
68+
69+
`auth_provider` is a separate property for that reason: it names which
70+
provider's sign-in was rejected (`codex`, `xai`), chosen from a fixed
71+
first-party set in `src/tui-opentui/session-chrome.ts`. No part of the
72+
provider's rejection message is sent.
6673

6774
The mapping is `src/telemetry/classify.ts`, and the tests that feed each
6875
emission site a deliberately identifying name and assert it reaches no part of

src/telemetry/index.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,9 @@ const EVENT_PROPERTY_ALLOWLIST: Record<TelemetryEvent, readonly string[]> = {
101101
permission_prompt: ["decision", "permission_kind"],
102102
compaction: ["mode", "duration_ms", "turns_before", "turns_after"],
103103
crash: ["kind", "error_class"],
104-
auth_failure: ["error_class"],
104+
// Which provider rejected the credentials, not why — the rejection detail is
105+
// provider-authored text and error_class means a JS constructor name.
106+
auth_failure: ["auth_provider"],
105107
};
106108

107109
const KNOWN_EVENTS: ReadonlySet<string> = new Set(Object.keys(EVENT_PROPERTY_ALLOWLIST));

src/tui-opentui/session-chrome.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
* duplicating the state machine that produces it.
66
*/
77

8+
import type { Telemetry } from "../telemetry/index.js"
89
import type { RampPhase } from "./ramp.js"
910

1011
/** Agent lifecycle status the progress label reads (mirrors the stream state). */
@@ -137,6 +138,20 @@ export function shouldSettleUiAfterSendFailure(kind: SendFailureKind): boolean {
137138
return kind === "codex_auth" || kind === "xai_auth" || kind === "error"
138139
}
139140

141+
// Send-failure kinds are first-party constants, so the provider each one names
142+
// is a fixed mapping rather than a classification over author-chosen text.
143+
const AUTH_FAILURE_PROVIDERS: Partial<Record<SendFailureKind, string>> = {
144+
codex_auth: "codex",
145+
xai_auth: "xai",
146+
}
147+
148+
/** Report which provider rejected the stored credentials; silent otherwise. */
149+
export function captureAuthFailure(telemetry: Telemetry, kind: SendFailureKind): void {
150+
const provider = AUTH_FAILURE_PROVIDERS[kind]
151+
if (provider === undefined) return
152+
telemetry.capture("auth_failure", { auth_provider: provider })
153+
}
154+
140155
// The stream carries a failure as a bare message string, so the auth errors are
141156
// recognised by the profile phrase their constructors always produce
142157
// (`Codex profile "default" is not authorized. …`).

src/tui/runner.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ import {
145145
surfaceSystemNotice,
146146
} from "../tui-opentui/shell.js";
147147
import {
148+
captureAuthFailure,
148149
classifyAgentSendFailure,
149150
shouldSettleUiAfterSendFailure,
150151
} from "../tui-opentui/session-chrome.js";
@@ -1772,9 +1773,7 @@ export async function runTUI(initialConfig: Config): Promise<number> {
17721773
isCodexAuthError,
17731774
isXaiAuthError,
17741775
);
1775-
if (kind === "codex_auth" || kind === "xai_auth") {
1776-
getTelemetry().capture("auth_failure", { error_class: kind });
1777-
}
1776+
captureAuthFailure(getTelemetry(), kind);
17781777
if (!shouldSettleUiAfterSendFailure(kind)) return;
17791778
recordRunError(err);
17801779
systemNotice(err instanceof Error ? err.message : String(err));

tests/unit/telemetry-product-events.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ import {
2323
classifyPermissionKind,
2424
} from "../../src/telemetry/classify.js";
2525
import { createTelemetry, type Telemetry } from "../../src/telemetry/index.js";
26+
import {
27+
captureAuthFailure,
28+
classifyAgentSendFailure,
29+
} from "../../src/tui-opentui/session-chrome.js";
2630

2731
type BatchBody = {
2832
batch: { event: string; properties: Record<string, unknown> }[];
@@ -256,6 +260,41 @@ test("crash reports language error types by name and buckets everything else", a
256260
expect(await wire()).not.toContain("AcmeCorp");
257261
});
258262

263+
// ---------------------------------------------------------------------------
264+
// auth_failure — the provider's rejection message names the profile
265+
// ---------------------------------------------------------------------------
266+
267+
test("auth_failure names the provider and never ships the rejection message", async () => {
268+
const { telemetry, wire, events } = harness();
269+
const isCodexAuth = (e: unknown) => e instanceof Error && /codex profile/i.test(e.message);
270+
const isXaiAuth = (e: unknown) => e instanceof Error && /xai profile/i.test(e.message);
271+
272+
const codexRejection = new Error('Codex profile "acmecorp-eng" is not authorized.');
273+
const rejections = [
274+
codexRejection,
275+
new Error('xai profile "acmecorp-eng" is not authorized.'),
276+
new Error("connection reset by /Users/someone/acmecorp"),
277+
];
278+
for (const err of rejections) {
279+
captureAuthFailure(
280+
telemetry,
281+
classifyAgentSendFailure(err, false, isCodexAuth, isXaiAuth),
282+
);
283+
}
284+
// An aborted send outranks the auth match, so it must emit nothing.
285+
captureAuthFailure(
286+
telemetry,
287+
classifyAgentSendFailure(codexRejection, true, isCodexAuth, isXaiAuth),
288+
);
289+
290+
const captured = await events();
291+
expect(captured.map((e) => e.event)).toEqual(["auth_failure", "auth_failure"]);
292+
expect(captured.map((e) => e.properties.auth_provider)).toEqual(["codex", "xai"]);
293+
const body = await wire();
294+
expect(body).not.toContain("acmecorp");
295+
expect(body).not.toContain("error_class");
296+
});
297+
259298
// ---------------------------------------------------------------------------
260299
// compaction — must not fire when the compactor did nothing
261300
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)