From 97178c581d4a01bfcf87dd2b12d862a014509592 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 17:20:48 -0700 Subject: [PATCH 01/24] feat(drift): add DriftClass enum, QuarantineEntry, per-item id/class, and quarantine[] --- scripts/drift-types.ts | 58 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/scripts/drift-types.ts b/scripts/drift-types.ts index 5eaec247..71f0d93d 100644 --- a/scripts/drift-types.ts +++ b/scripts/drift-types.ts @@ -15,6 +15,29 @@ */ export type DriftSeverity = "critical" | "warning" | "info"; +/** + * Coarse drift-classification enum used by the delta/summary layers to route a + * report to the right terminal outcome (block vs advisory vs quarantine). It is + * orthogonal to per-diff `DriftSeverity`: a report is `Quarantine` when its + * findings could not be trusted (unparseable / unmapped surface), independent of + * whether individual diffs were `critical`. Purely additive — existing consumers + * that never read `class` are unaffected. + */ +export enum DriftClass { + /** At least one critical, trustworthy drift finding — hard failure. */ + Critical = "critical", + /** Non-critical, informational drift — advisory only. */ + Advisory = "advisory", + /** + * A failure that could not be parsed/mapped into a trustworthy drift finding. + * Neither a clean pass nor a confirmed critical — held aside for human review + * so it is never silently swallowed as a green. + */ + Quarantine = "quarantine", + /** No drift detected. */ + None = "none", +} + export interface ParsedDiff { path: string; severity: DriftSeverity; @@ -22,6 +45,35 @@ export interface ParsedDiff { expected: string; real: string; mock: string; + /** + * Optional stable per-item key (e.g. a model id) used by the delta layer to + * key findings by provider+id. Absent on legacy diffs. + */ + id?: string; + /** Optional coarse classification for this diff. Absent on legacy diffs. */ + class?: DriftClass; +} + +/** + * A test failure that could not be parsed into a trustworthy drift finding and + * was NOT positively classified as benign infrastructure. Rather than crash the + * collector (exit 1) or silently drop the failure (exit 0), the failure is + * captured here so it can surface as a distinct quarantine outcome (exit 5) for + * human review. + */ +export interface QuarantineEntry { + /** Provider inferred from the failing assertion, or a best-effort label. */ + provider: string; + /** The failing test's name (ancestor titles + title). */ + testName: string; + /** + * Raw `file:line` captured from the original stack frame BEFORE stack-frame + * stripping, so the human reviewer can locate the failing assertion. Empty + * string when no frame was available. + */ + rawLocation: string; + /** The (possibly truncated) failure message that could not be parsed. */ + message: string; } export interface DriftEntry { @@ -37,4 +89,10 @@ export interface DriftEntry { export interface DriftReport { timestamp: string; entries: DriftEntry[]; + /** + * Optional list of failures held aside for human review (see QuarantineEntry). + * Absent/empty when there was nothing to quarantine — legacy consumers that + * ignore this field are unaffected. + */ + quarantine?: QuarantineEntry[]; } From 66f575dbb1cf8909a640e69df5e4cc4b535c69c5 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 17:21:44 -0700 Subject: [PATCH 02/24] feat(drift): add pure computeExitCode and route main() through it (quarantine=5) --- scripts/drift-report-collector.ts | 66 +++++++++++++++++++++++++------ 1 file changed, 55 insertions(+), 11 deletions(-) diff --git a/scripts/drift-report-collector.ts b/scripts/drift-report-collector.ts index 673fb7fb..7cd8788e 100644 --- a/scripts/drift-report-collector.ts +++ b/scripts/drift-report-collector.ts @@ -10,7 +10,8 @@ * Exit codes: * 0 — no critical diffs found (or no drift at all) * 2 — at least one critical diff exists - * 1 — script error (unhandled exception) + * 5 — at least one failure was quarantined (unparseable/untrusted — needs review) + * 1 — AG-UI drift detection was skipped (infra), or an unhandled script error * * Usage: * npx tsx scripts/drift-report-collector.ts [--out drift-report.json] @@ -984,6 +985,39 @@ function collectAgUiDriftEntries(results: VitestJsonResult): DriftEntry[] { return entries; } +// --------------------------------------------------------------------------- +// Exit-code policy +// --------------------------------------------------------------------------- + +/** + * Map the three terminal signals a drift run can produce onto the collector's + * process exit code. Pure and side-effect-free so the mapping is unit-testable + * without spawning the drift suite. + * + * Precedence (highest first): + * - `criticalCount > 0` → 2 — at least one trustworthy critical drift. + * - `quarantineCount > 0`→ 5 — a failure could not be parsed/mapped into a + * trustworthy finding and was held for review; + * distinct from a critical (2) and from a clean + * pass (0) so it is never silently swallowed. + * - `agUiSkipped` → 1 — AG-UI drift detection could not run (infra). + * O2: AG-UI-skipped stays exit 1 (unchanged). + * - otherwise → 0 — no drift. + * + * Critical wins over quarantine: if the run found a genuine critical drift, that + * is the actionable signal even if some other failure was also quarantined. + */ +export function computeExitCode( + criticalCount: number, + quarantineCount: number, + agUiSkipped: boolean, +): 0 | 1 | 2 | 5 { + if (criticalCount > 0) return 2; + if (quarantineCount > 0) return 5; + if (agUiSkipped) return 1; + return 0; +} + // --------------------------------------------------------------------------- // Main // --------------------------------------------------------------------------- @@ -1042,17 +1076,27 @@ function main(): void { ); console.log(` Critical diffs: ${criticalCount}`); - if (criticalCount > 0) { - console.log("Exiting with code 2 (critical diffs found)."); - process.exit(2); + // Quarantine count is wired in A1.3 (collectDriftEntries appends quarantine + // entries instead of throwing). Until then there are none to report. + const quarantineCount = 0; + + const exitCode = computeExitCode(criticalCount, quarantineCount, agUiSkipped); + switch (exitCode) { + case 2: + console.log("Exiting with code 2 (critical diffs found)."); + process.exit(2); + // eslint-disable-next-line no-fallthrough + case 5: + console.warn(`Exiting with code 5 (${quarantineCount} failure(s) quarantined for review).`); + process.exit(5); + // eslint-disable-next-line no-fallthrough + case 1: + console.warn("Exiting with code 1 (AG-UI drift detection was skipped — infra failure)."); + process.exit(1); + // eslint-disable-next-line no-fallthrough + default: + console.log("No critical diffs found."); } - - if (agUiSkipped) { - console.warn("Exiting with code 1 (AG-UI drift detection was skipped — infra failure)."); - process.exit(1); - } - - console.log("No critical diffs found."); } /** From 2dd48f7e74e9b51297e705024f0514ff43aeca73 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 17:21:48 -0700 Subject: [PATCH 03/24] feat(drift): add shared normalizeModelFamily primitive Extract the OpenAI voice-model dated-snapshot/build-tag family-strip loop into a side-effect-free normalizeModelFamily(id, provider) shared core. The provider argument is a forward hook; the strip is applied identically for all three providers today, so normalizeModelFamily(id, "openai") is byte-identical to normalizeVoiceModelFamily(id). --- src/__tests__/drift/model-family.test.ts | 19 ++++++++++ src/__tests__/drift/model-family.ts | 44 ++++++++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 src/__tests__/drift/model-family.test.ts create mode 100644 src/__tests__/drift/model-family.ts diff --git a/src/__tests__/drift/model-family.test.ts b/src/__tests__/drift/model-family.test.ts new file mode 100644 index 00000000..de4ce61c --- /dev/null +++ b/src/__tests__/drift/model-family.test.ts @@ -0,0 +1,19 @@ +/** + * Unit test for the shared `normalizeModelFamily` primitive. + */ +import { describe, it, expect } from "vitest"; +import { normalizeModelFamily } from "./model-family.js"; + +describe("normalizeModelFamily", () => { + it("strips a trailing dated snapshot suffix", () => { + expect(normalizeModelFamily("gpt-audio-2025-08-28", "openai")).toBe("gpt-audio"); + }); + + it("strips a trailing build-tag suffix", () => { + expect(normalizeModelFamily("tts-1-1106", "openai")).toBe("tts-1"); + }); + + it("does not strip a single-digit suffix", () => { + expect(normalizeModelFamily("gpt-live-1", "openai")).toBe("gpt-live-1"); + }); +}); diff --git a/src/__tests__/drift/model-family.ts b/src/__tests__/drift/model-family.ts new file mode 100644 index 00000000..75b9c17e --- /dev/null +++ b/src/__tests__/drift/model-family.ts @@ -0,0 +1,44 @@ +/** + * Shared, side-effect-free `normalizeModelFamily` primitive (no + * `describe`/`beforeAll`) reducing a model id to its FAMILY KEY by stripping the + * trailing version/snapshot suffixes that providers append to already-known + * families. + * + * New dated snapshots of an existing family land constantly (`tts-1-1106`, + * `gpt-audio-2025-08-28`, `gpt-4o-mini-tts-2025-12-15`, …); appending every one + * to a known-ID set never converges and turns the daily drift job permanently + * red on false positives. Comparing the NORMALIZED family instead means only a + * genuinely new family (e.g. `gpt-live`) is ever flagged. + * + * Two suffix shapes are stripped, repeatedly, from the END of the id: + * - a dated snapshot `-YYYY-MM-DD` (e.g. `-2025-08-28`) + * - a build/version tag `-NNN` or `-NNNN` (3–4 digits, e.g. `-1106`) + * + * Both are anchored to the end and applied in a loop so a trailing dated + * snapshot that itself follows a build tag is fully reduced. A short numeric + * suffix like `gpt-live-1`'s trailing `-1` is a SINGLE digit and is deliberately + * NOT stripped, so `gpt-live-1` normalizes to `gpt-live-1` — an unknown family — + * and stays flagged (the whole point of the canary). + * + * The `provider` argument is a forward hook: the dated-snapshot/build-tag strip + * below is the SHARED CORE applied identically for all three providers today, so + * `normalizeModelFamily(id, "openai")` is byte-identical to the historical + * `normalizeVoiceModelFamily(id)`. Provider-specific normalization can branch off + * `provider` later without touching call sites. + */ +const DATED_SNAPSHOT_SUFFIX = /-\d{4}-\d{2}-\d{2}$/; +const BUILD_TAG_SUFFIX = /-\d{3,4}$/; + +export function normalizeModelFamily( + id: string, + provider: "openai" | "anthropic" | "gemini", +): string { + void provider; + let family = id; + for (;;) { + const stripped = family.replace(DATED_SNAPSHOT_SUFFIX, "").replace(BUILD_TAG_SUFFIX, ""); + if (stripped === family) break; + family = stripped; + } + return family; +} From 854c4e614fc83ccc00c7a7cd0bc9ec46ab58b4e2 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 17:23:52 -0700 Subject: [PATCH 04/24] refactor(drift): reimplement normalizeVoiceModelFamily over shared primitive normalizeVoiceModelFamily(id) is now a thin wrapper over normalizeModelFamily(id, "openai"); the duplicated dated-snapshot/build-tag strip loop moves to the shared model-family primitive. Behavior-preserving: knownVoiceModelFamilies is byte-identical and all voice-model unit tests plus the ws-realtime drift selection stay green. Adds a characterization test asserting the wrapper is byte-identical to the shared primitive. --- src/__tests__/drift/voice-models.ts | 42 +++++++++--------------- src/__tests__/ws-realtime-canary.test.ts | 15 +++++++++ 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/src/__tests__/drift/voice-models.ts b/src/__tests__/drift/voice-models.ts index e1e28166..57b24895 100644 --- a/src/__tests__/drift/voice-models.ts +++ b/src/__tests__/drift/voice-models.ts @@ -10,6 +10,8 @@ * drift suite (which would spin up the drift server). */ +import { normalizeModelFamily } from "./model-family.js"; + /** * The GA realtime model FAMILIES. At least one voice/audio model whose * normalized family (see `normalizeVoiceModelFamily`) is one of these MUST @@ -28,35 +30,23 @@ export const gaRealtimeModels = [ ]; /** - * Normalize a model id to its FAMILY KEY by stripping trailing version/snapshot - * suffixes that OpenAI appends to already-known families. New dated snapshots of - * an existing family land constantly (`tts-1-1106`, `gpt-audio-2025-08-28`, - * `gpt-4o-mini-tts-2025-12-15`, …); appending every one to a known-ID set never - * converges and turns the daily drift job permanently red on false positives. - * Comparing the NORMALIZED family instead means only a genuinely new family - * (e.g. `gpt-live`) is ever flagged. - * - * Two suffix shapes are stripped, repeatedly, from the END of the id: - * - a dated snapshot `-YYYY-MM-DD` (e.g. `-2025-08-28`) - * - a build/version tag `-NNN` or `-NNNN` (3–4 digits, e.g. `-1106`) + * Normalize a voice/audio model id to its FAMILY KEY by stripping trailing + * version/snapshot suffixes that OpenAI appends to already-known families. New + * dated snapshots of an existing family land constantly (`tts-1-1106`, + * `gpt-audio-2025-08-28`, `gpt-4o-mini-tts-2025-12-15`, …); appending every one + * to a known-ID set never converges and turns the daily drift job permanently + * red on false positives. Comparing the NORMALIZED family instead means only a + * genuinely new family (e.g. `gpt-live`) is ever flagged. * - * Both are anchored to the end and applied in a loop so a trailing dated - * snapshot that itself follows a build tag is fully reduced. A short numeric - * suffix like `gpt-live-1`'s trailing `-1` is a SINGLE digit and is deliberately - * NOT stripped, so `gpt-live-1` normalizes to `gpt-live-1` — an unknown family — - * and stays flagged (the whole point of the canary). + * This is a thin OpenAI-provider wrapper over the shared `normalizeModelFamily` + * primitive (see `model-family.ts`), which owns the dated-snapshot/build-tag + * strip loop. Behavior is byte-identical to the historical inline implementation + * — a short numeric suffix like `gpt-live-1`'s trailing `-1` is a SINGLE digit + * and is deliberately NOT stripped, so `gpt-live-1` normalizes to `gpt-live-1` + * — an unknown family — and stays flagged (the whole point of the canary). */ -const DATED_SNAPSHOT_SUFFIX = /-\d{4}-\d{2}-\d{2}$/; -const BUILD_TAG_SUFFIX = /-\d{3,4}$/; - export function normalizeVoiceModelFamily(id: string): string { - let family = id; - for (;;) { - const stripped = family.replace(DATED_SNAPSHOT_SUFFIX, "").replace(BUILD_TAG_SUFFIX, ""); - if (stripped === family) break; - family = stripped; - } - return family; + return normalizeModelFamily(id, "openai"); } /** diff --git a/src/__tests__/ws-realtime-canary.test.ts b/src/__tests__/ws-realtime-canary.test.ts index 7a6cacd1..ac61ee1a 100644 --- a/src/__tests__/ws-realtime-canary.test.ts +++ b/src/__tests__/ws-realtime-canary.test.ts @@ -21,6 +21,7 @@ import { knownVoiceModelFamilies, normalizeVoiceModelFamily, } from "./drift/voice-models.js"; +import { normalizeModelFamily } from "./drift/model-family.js"; // Dated-snapshot / build-tag variants of families that are ALREADY known. These // are the exact ids the live Drift Tests run (28968203340) flagged as false @@ -126,6 +127,20 @@ describe("ws-realtime known-voice-models canary detection", () => { expect(knownVoiceModelFamilies.has("gpt-live-1")).toBe(false); }); + it("is byte-identical to the shared normalizeModelFamily(id, 'openai') primitive", () => { + // Characterization guard for the A3.2 equivalence refactor: normalizeVoiceModelFamily + // is now a thin wrapper over normalizeModelFamily(id, "openai"), and the migrated + // wrapper must yield the SAME normalization for every representative id AND an + // identical knownVoiceModelFamilies set as re-seeding through the shared primitive. + for (const id of [...REPRESENTATIVE_MODELS, ...knownVoiceModelFamilies]) { + expect(normalizeVoiceModelFamily(id)).toBe(normalizeModelFamily(id, "openai")); + } + const reseeded = new Set( + [...knownVoiceModelFamilies].map((f) => normalizeModelFamily(f, "openai")), + ); + expect([...knownVoiceModelFamilies].sort()).toEqual([...reseeded].sort()); + }); + it("does not flag non-voice chat/image/embedding models as voice drift", () => { const { candidateModels, unknown } = detectVoiceModelDrift(REPRESENTATIVE_MODELS); From 4a388f2455c55268859a14f9bc087d9fcda21673 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 17:27:37 -0700 Subject: [PATCH 05/24] feat(drift): quarantine unmapped/unparseable-not-infra failures instead of throwing (exit 5) --- scripts/drift-report-collector.ts | 131 ++++++++++++++++++++++++------ 1 file changed, 108 insertions(+), 23 deletions(-) diff --git a/scripts/drift-report-collector.ts b/scripts/drift-report-collector.ts index 7cd8788e..26417a3f 100644 --- a/scripts/drift-report-collector.ts +++ b/scripts/drift-report-collector.ts @@ -22,7 +22,13 @@ import { existsSync, statSync, writeFileSync } from "node:fs"; import { resolve } from "node:path"; import { fileURLToPath } from "node:url"; -import type { DriftEntry, DriftReport, DriftSeverity, ParsedDiff } from "./drift-types.js"; +import type { + DriftEntry, + DriftReport, + DriftSeverity, + ParsedDiff, + QuarantineEntry, +} from "./drift-types.js"; // --------------------------------------------------------------------------- // Vitest JSON reporter types (subset we care about) @@ -597,6 +603,34 @@ function stripStackFrames(msg: string): string { .join("\n"); } +/** + * O-1: capture the raw `file:line` (or `file:line:col`) from the FIRST usable + * stack frame BEFORE `stripStackFrames` removes it, so a quarantined failure + * carries a pointer the human reviewer can jump to. Prefers a project-source + * frame (`src/…`) over node_modules/internal frames; falls back to the first + * frame with any `path:line` shape. Returns "" when no frame is present. + */ +export function extractRawLocation(msg: string): string { + const frames = msg.split("\n").filter((line) => /^\s*at\s/.test(line)); + // Match `path:line` or `path:line:col`, with an optional trailing `)`. + const locRe = /((?:\/|\.\/|[A-Za-z]:\\|file:\/\/)?[^\s()]+?:\d+(?::\d+)?)\)?\s*$/; + const pick = (predicate: (f: string) => boolean): string | null => { + for (const frame of frames) { + if (!predicate(frame)) continue; + const m = frame.match(locRe); + if (m) return m[1]; + } + return null; + }; + // Prefer a project-source frame, skipping node internals / node_modules. + return ( + pick((f) => /src\//.test(f) && !/node_modules/.test(f) && !/node:internal/.test(f)) ?? + pick((f) => !/node_modules/.test(f) && !/node:internal/.test(f)) ?? + pick(() => true) ?? + "" + ); +} + export function classifyUnparseableAsInfra(unparseableMessages: string[]): boolean { // CLASS 1 — fail-loud on absent evidence. No messages means NO positive infra // evidence, so this is NOT a benign "all clear". `[].every(...)` is vacuously @@ -618,9 +652,22 @@ export function classifyUnparseableAsInfra(unparseableMessages: string[]): boole return allInfraErrors && !anyDriftLike; } -export function collectDriftEntries(results: VitestJsonResult): DriftEntry[] { +/** + * The result of collecting drift entries. In addition to the trustworthy drift + * `entries`, `quarantine` holds failures that could not be parsed/mapped into a + * trustworthy finding AND were not positively classified as benign infra (A1.3). + * These no longer crash the collector (exit 1) nor get silently dropped (exit + * 0): the caller routes a non-empty `quarantine` to exit 5. Exit 1 is now + * reserved for genuine collector bugs (unhandled exceptions). + */ +export interface CollectResult { + entries: DriftEntry[]; + quarantine: QuarantineEntry[]; +} + +export function collectDriftEntries(results: VitestJsonResult): CollectResult { const entries: DriftEntry[] = []; - const unmapped: string[] = []; + const quarantine: QuarantineEntry[] = []; let unparseable = 0; for (const file of results.testResults) { @@ -731,15 +778,30 @@ export function collectDriftEntries(results: VitestJsonResult): DriftEntry[] { // Determine provider from ancestor titles (describe block) or context const ancestorText = assertion.ancestorTitles.join(" "); + const testName = `${ancestorText} > ${assertion.title}`; const provider = extractProviderName(ancestorText) ?? extractProviderName(parsed.context); if (!provider) { - unmapped.push(`${ancestorText} > ${assertion.title}`); + // Unmapped provider: a parseable drift block whose provider we cannot + // route to a source file. Held for review (exit 5) rather than crashing + // the whole run (was exit 1). O-1: capture the raw frame location BEFORE + // any stack stripping. + quarantine.push({ + provider: parsed.context || ancestorText || "unknown", + testName, + rawLocation: extractRawLocation(fullMessage), + message: fullMessage, + }); continue; } const mapping = PROVIDER_MAP[provider]; if (!mapping) { - unmapped.push(`${ancestorText} > ${assertion.title} (provider: ${provider})`); + quarantine.push({ + provider, + testName, + rawLocation: extractRawLocation(fullMessage), + message: fullMessage, + }); continue; } @@ -755,26 +817,38 @@ export function collectDriftEntries(results: VitestJsonResult): DriftEntry[] { } } - if (unmapped.length > 0) { - console.error(`ERROR: ${unmapped.length} drift failure(s) could not be mapped to a provider:`); - for (const u of unmapped) console.error(` - ${u}`); - throw new Error(`${unmapped.length} unmapped drift entries — update PROVIDER_MAP`); + if (quarantine.length > 0) { + console.warn( + `WARNING: ${quarantine.length} drift failure(s) could not be mapped to a provider — ` + + `quarantined for review (exit 5), not crashed:`, + ); + for (const q of quarantine) + console.warn(` - ${q.testName} @ ${q.rawLocation || ""}`); } if (unparseable > 0 && entries.length === 0) { - // Collect the unparseable failure messages to classify them - const unparseableMessages: string[] = []; + // Collect the unparseable failure messages (with their raw pre-strip + // locations) to classify them. O-1: capture file:line BEFORE stripping. + const unparseableFailures: { message: string; testName: string; rawLocation: string }[] = []; for (const file of results.testResults) { for (const assertion of file.assertionResults) { if (assertion.status !== "failed" || assertion.failureMessages.length === 0) continue; const fullMessage = assertion.failureMessages.join("\n"); const parsed = parseDriftBlock(fullMessage); if (!parsed || parsed.diffs.length === 0) { - unparseableMessages.push(fullMessage); + // Canary shapes are handled above (they became entries) — only truly + // unparseable messages reach here. + if (parseKnownModelsCanary(fullMessage) !== null) continue; + unparseableFailures.push({ + message: fullMessage, + testName: `${assertion.ancestorTitles.join(" ")} > ${assertion.title}`, + rawLocation: extractRawLocation(fullMessage), + }); } } } + const unparseableMessages = unparseableFailures.map((f) => f.message); for (const msg of unparseableMessages) { console.warn(` Unparseable failure message (first 300 chars): ${msg.slice(0, 300)}`); } @@ -785,13 +859,22 @@ export function collectDriftEntries(results: VitestJsonResult): DriftEntry[] { `(not drift reports). Continuing with 0 drift entries.`, ); } else { - console.error( - `ERROR: ${unparseable} test failure(s) could not be parsed as drift reports.`, - "This may indicate broken test infrastructure or a changed report format.", - ); - throw new Error( - `${unparseable} unparseable test failures with 0 drift entries — investigate`, + // A1.3: genuine-but-unparseable drift is no longer a fail-loud crash (exit + // 1). Each such failure is quarantined (exit 5) so it surfaces for human + // review without being silently swallowed as a green. Exit 1 is now + // reserved for genuine collector bugs (unhandled exceptions). + console.warn( + `WARNING: ${unparseable} test failure(s) could not be parsed as drift reports — ` + + `quarantined for review (exit 5).`, ); + for (const f of unparseableFailures) { + quarantine.push({ + provider: "unknown", + testName: f.testName, + rawLocation: f.rawLocation, + message: f.message, + }); + } } } else if (unparseable > 0) { console.warn( @@ -799,7 +882,7 @@ export function collectDriftEntries(results: VitestJsonResult): DriftEntry[] { ); } - return entries; + return { entries, quarantine }; } // --------------------------------------------------------------------------- @@ -1033,7 +1116,8 @@ function main(): void { console.log("Running HTTP API drift tests..."); const httpResults = runDriftTests(); console.log("Collecting HTTP API drift entries..."); - const httpEntries = collectDriftEntries(httpResults); + const httpResult = collectDriftEntries(httpResults); + const httpEntries = httpResult.entries; // Collect AG-UI schema drift entries console.log("Running AG-UI schema drift tests..."); @@ -1048,10 +1132,12 @@ function main(): void { } const entries = [...httpEntries, ...agUiEntries]; + const quarantine = httpResult.quarantine; const report: DriftReport = { timestamp: new Date().toISOString(), entries, + ...(quarantine.length > 0 ? { quarantine } : {}), }; try { @@ -1076,9 +1162,8 @@ function main(): void { ); console.log(` Critical diffs: ${criticalCount}`); - // Quarantine count is wired in A1.3 (collectDriftEntries appends quarantine - // entries instead of throwing). Until then there are none to report. - const quarantineCount = 0; + const quarantineCount = quarantine.length; + console.log(` Quarantined failures: ${quarantineCount}`); const exitCode = computeExitCode(criticalCount, quarantineCount, agUiSkipped); switch (exitCode) { From 5300f6b8bf2b8316144995406775992e9c2bb3a2 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 17:27:41 -0700 Subject: [PATCH 06/24] test(drift): exit-code taxonomy + quarantine harness for computeExitCode/collector (A1.4) --- src/__tests__/drift-collector.test.ts | 350 ++++++++++++++++++++++---- 1 file changed, 297 insertions(+), 53 deletions(-) diff --git a/src/__tests__/drift-collector.test.ts b/src/__tests__/drift-collector.test.ts index 6bf2d672..4eeabc47 100644 --- a/src/__tests__/drift-collector.test.ts +++ b/src/__tests__/drift-collector.test.ts @@ -24,10 +24,36 @@ import { extractScenario, parseKnownModelsCanary, collectDriftEntries, + computeExitCode, classifyUnparseableAsInfra, INFRA_INDICATOR_SOURCES, infraIndicatorSample, } from "../../scripts/drift-report-collector.js"; +import type { DriftEntry, QuarantineEntry } from "../../scripts/drift-types.js"; + +// --------------------------------------------------------------------------- +// Helpers for the A1.3 CollectResult shape ({ entries, quarantine }). +// collectDriftEntries no longer returns a bare array nor throws on unmapped / +// unparseable-not-infra failures — those are quarantined (→ exit 5). +// --------------------------------------------------------------------------- + +function entriesOf(result: VitestJsonResult): DriftEntry[] { + return collectDriftEntries(result).entries; +} + +function quarantineOf(result: VitestJsonResult): QuarantineEntry[] { + return collectDriftEntries(result).quarantine; +} + +/** The exit code main() would emit for a given collect result (agUiSkipped=false). */ +function exitCodeOf(result: VitestJsonResult): 0 | 1 | 2 | 5 { + const { entries, quarantine } = collectDriftEntries(result); + const criticalCount = entries.reduce( + (sum, e) => sum + e.diffs.filter((d) => d.severity === "critical").length, + 0, + ); + return computeExitCode(criticalCount, quarantine.length, false); +} // --------------------------------------------------------------------------- // Vitest JSON reporter fixture builders @@ -319,19 +345,25 @@ describe("extractScenario", () => { // --------------------------------------------------------------------------- describe("collectDriftEntries", () => { - it("returns empty array when no failed tests", () => { + it("returns empty entries+quarantine when no failed tests", () => { const result = makeResult([ makeAssertion({ status: "passed" }), makeAssertion({ status: "pending" }), ]); - expect(collectDriftEntries(result)).toEqual([]); + expect(entriesOf(result)).toEqual([]); + expect(quarantineOf(result)).toEqual([]); + expect(exitCodeOf(result)).toBe(0); }); - it("returns empty array when there are no test files at all", () => { - expect(collectDriftEntries({ testResults: [] })).toEqual([]); + it("returns empty entries+quarantine when there are no test files at all", () => { + const result: VitestJsonResult = { testResults: [] }; + expect(entriesOf(result)).toEqual([]); + expect(quarantineOf(result)).toEqual([]); }); - it("throws when an unmapped provider is found in drift report", () => { + it("QUARANTINES (does NOT throw) an unmapped provider found in a drift report → exit 5", () => { + // A1.3: an unmapped provider is untrusted, not a collector crash. It is held + // for review (exit 5) instead of throwing (was exit 1). const driftText = formatDriftReport("UnknownProvider (non-streaming text)", [SAMPLE_DIFF]); const result = makeResult([ makeAssertion({ @@ -340,21 +372,43 @@ describe("collectDriftEntries", () => { failureMessages: [driftText], }), ]); - expect(() => collectDriftEntries(result)).toThrow(/unmapped drift entries/); + expect(() => collectDriftEntries(result)).not.toThrow(); + const q = quarantineOf(result); + expect(q).toHaveLength(1); + expect(entriesOf(result)).toEqual([]); + expect(exitCodeOf(result)).toBe(5); }); - it("throws when all failures are unparseable and no drift entries collected", () => { + it("QUARANTINES (does NOT throw) all-unparseable-not-infra failures → exit 5 (incident-5)", () => { + // A1.3: the incident-5 surface. Genuine-but-unparseable failures are no + // longer a fail-loud crash (exit 1) — they are quarantined (exit 5) so they + // surface for review without being swallowed. const result = makeResult([ makeAssertion({ status: "failed", - failureMessages: ["Error: expected true to equal false\n at Object."], + ancestorTitles: ["Some Suite"], + title: "a", + failureMessages: [ + "AssertionError: expected 1 to be 2 // Object.is equality\n at foo (/repo/src/__tests__/drift/some.drift.ts:8:13)", + ], }), makeAssertion({ status: "failed", - failureMessages: ["TypeError: Cannot read property 'foo' of undefined"], + ancestorTitles: ["Other Suite"], + title: "b", + failureMessages: [ + "TypeError: Cannot read property 'foo' of undefined\n at bar (/repo/src/__tests__/drift/other.drift.ts:3:1)", + ], }), ]); - expect(() => collectDriftEntries(result)).toThrow(/unparseable test failures/); + expect(() => collectDriftEntries(result)).not.toThrow(); + const q = quarantineOf(result); + expect(q).toHaveLength(2); + // O-1: raw file:line captured from the stack frame BEFORE stripping. + expect(q[0].rawLocation).toBe("/repo/src/__tests__/drift/some.drift.ts:8:13"); + expect(q[1].rawLocation).toBe("/repo/src/__tests__/drift/other.drift.ts:3:1"); + expect(entriesOf(result)).toEqual([]); + expect(exitCodeOf(result)).toBe(5); }); it("returns valid entries and tolerates unparseable failures mixed in", () => { @@ -374,13 +428,19 @@ describe("collectDriftEntries", () => { }), ]); - const entries = collectDriftEntries(result); + const entries = entriesOf(result); expect(entries).toHaveLength(1); expect(entries[0].provider).toBe("OpenAI Chat"); expect(entries[0].scenario).toBe("non-streaming text"); expect(entries[0].builderFile).toBe("src/helpers.ts"); expect(entries[0].diffs).toHaveLength(1); expect(entries[0].diffs[0].severity).toBe("critical"); + // When real drift entries ARE present, a mixed-in unparseable sibling stays + // TOLERATED (warn-only) rather than quarantined — the quarantine path only + // fires for the all-unparseable, zero-entries case (former throw site). + expect(quarantineOf(result)).toEqual([]); + // A critical entry present → exit 2 (dominates any tolerated sibling). + expect(exitCodeOf(result)).toBe(2); }); it("ignores passed assertions in a mixed result set", () => { @@ -395,7 +455,7 @@ describe("collectDriftEntries", () => { }), ]); - const entries = collectDriftEntries(result); + const entries = entriesOf(result); expect(entries).toHaveLength(1); expect(entries[0].provider).toBe("OpenAI Chat"); }); @@ -429,7 +489,7 @@ describe("collectDriftEntries", () => { ], }; - const entries = collectDriftEntries(results); + const entries = entriesOf(results); expect(entries).toHaveLength(2); expect(entries[0].provider).toBe("OpenAI Chat"); expect(entries[1].provider).toBe("Google Gemini"); @@ -450,7 +510,7 @@ describe("collectDriftEntries", () => { }), ]); - const entries = collectDriftEntries(result); + const entries = entriesOf(result); expect(entries).toHaveLength(1); const entry = entries[0]; @@ -484,14 +544,16 @@ describe("collectDriftEntries", () => { expect(criticalCount).toBe(4); }); - it("does NOT misattribute a non-canary toEqual([]) failure from another provider as OpenAI-Realtime drift (RED without the gate)", () => { + it("does NOT misattribute a non-canary toEqual([]) failure from another provider as OpenAI-Realtime drift → quarantine (exit 5)", () => { // A different provider's test failed with the generic vitest shape // `expected [ 'sk-leaked' ] to deeply equal []`. Pre-fix, the unguarded // canary fallback matched this and emitted a CRITICAL "OpenAI Realtime // known-models canary" entry with `real: 'sk-leaked'`, pointing the auto-fix // at src/ws-realtime.ts and relabeling arbitrary array contents as a model - // id. It must NOT be claimed as a canary; with no other parseable/infra - // signal the collector must fail loud (throw) rather than fabricate an entry. + // id. It must NOT be claimed as a canary. A1.3: because the message is + // neither a parseable drift block, a canary, nor infra, it is QUARANTINED + // (exit 5) — never fabricated into a false entry, never silently dropped, + // and (A1.3) no longer a fail-loud crash. const NON_CANARY_TOEQUAL_EMPTY = "AssertionError: expected [ 'sk-leaked' ] to deeply equal []\n" + " at /repo/src/__tests__/drift/openai-chat.drift.ts:42:30\n" + @@ -505,9 +567,13 @@ describe("collectDriftEntries", () => { }), ]); - // No OpenAI-Realtime entry may be produced. Because the message is neither a - // parseable drift block, a canary, nor infra, the collector fails loud. - expect(() => collectDriftEntries(result)).toThrow(/unparseable test failures/); + expect(() => collectDriftEntries(result)).not.toThrow(); + // No OpenAI-Realtime (nor any) entry may be produced. + expect(entriesOf(result)).toEqual([]); + expect(quarantineOf(result)).toHaveLength(1); + // The arbitrary array content is NOT relabeled as a model id anywhere. + expect(quarantineOf(result)[0].message).toContain("sk-leaked"); + expect(exitCodeOf(result)).toBe(5); }); it("surfaces a genuine drift report carried in an AssertionError with a leading blank line (does not swallow)", () => { @@ -520,18 +586,19 @@ describe("collectDriftEntries", () => { }), ]); - const entries = collectDriftEntries(result); + const entries = entriesOf(result); expect(entries).toHaveLength(1); expect(entries[0].provider).toBe("OpenAI Chat"); expect(entries[0].diffs).toHaveLength(1); expect(entries[0].diffs[0].severity).toBe("critical"); }); - it("throws (does NOT exit 0) when the only failure is unparseable with an infra token confined to a stack frame (A3)", () => { - // RED on the pre-fix collector: the raw-vs-stripped asymmetry classified - // this as benign infra and swallowed it (returned []). The fix normalizes - // both scans, so an infra token that survives ONLY in a stripped-away stack - // frame no longer flips the gate — the failure is surfaced via throw. + it("does NOT exit 0 (quarantines → exit 5) when the only failure has an infra token confined to a stack frame (A3)", () => { + // The raw-vs-stripped asymmetry classified this as benign infra and swallowed + // it (returned []). The fix normalizes both scans, so an infra token that + // survives ONLY in a stripped-away stack frame no longer flips the gate — the + // failure is surfaced. A1.3: surfaced now means QUARANTINE (exit 5), not a + // crash; the invariant that matters is it is NEVER a green (exit 0). const result = makeResult([ makeAssertion({ status: "failed", @@ -540,7 +607,10 @@ describe("collectDriftEntries", () => { failureMessages: [INFRA_TOKEN_IN_STACKFRAME_ONLY], }), ]); - expect(() => collectDriftEntries(result)).toThrow(/unparseable test failures/); + expect(() => collectDriftEntries(result)).not.toThrow(); + expect(quarantineOf(result)).toHaveLength(1); + expect(exitCodeOf(result)).toBe(5); + expect(exitCodeOf(result)).not.toBe(0); }); // ------------------------------------------------------------------------- @@ -557,7 +627,7 @@ describe("collectDriftEntries", () => { }), ]); - const entries = collectDriftEntries(result); + const entries = entriesOf(result); expect(entries).toHaveLength(1); const entry = entries[0]; expect(entry.provider).toBe("OpenAI Realtime"); @@ -584,7 +654,7 @@ describe("collectDriftEntries", () => { failureMessages: [CANARY_NO_GA_WITH_UNKNOWN], }), ]); - const entries = collectDriftEntries(result); + const entries = entriesOf(result); expect(entries).toHaveLength(1); const entry = entries[0]; expect(entry.provider).toBe("OpenAI Realtime"); @@ -605,7 +675,7 @@ describe("collectDriftEntries", () => { failureMessages: [CANARY_NO_GA_EMPTY], }), ]); - const entries = collectDriftEntries(result); + const entries = entriesOf(result); expect(entries).toHaveLength(1); expect(entries[0].provider).toBe("OpenAI Realtime"); expect(entries[0].diffs.every((d) => d.severity === "critical")).toBe(true); @@ -624,7 +694,7 @@ describe("collectDriftEntries", () => { failureMessages: [CANARY_FALLBACK_TRUNCATED], }), ]); - const entries = collectDriftEntries(result); + const entries = entriesOf(result); expect(entries).toHaveLength(1); for (const d of entries[0].diffs) { // No `real` value may be a prose annotation (e.g. "(additional models…)"). @@ -634,10 +704,16 @@ describe("collectDriftEntries", () => { // ------------------------------------------------------------------------- // CLASS 1 — corpus/table test asserting the SAFE outcome for the recurring - // classifier failure shapes. `throws: true` = fail-loud (exit-1 investigate); - // `throws: false` = surfaced as a structured entry (never a silent exit-0). + // classifier failure shapes. A1.3 replaces the old binary throws/no-throw + // with a three-way `outcome`: + // "entry" → surfaced as a structured drift entry (exit 2), NEVER a + // silent exit-0; + // "quarantine" → held for human review (exit 5), NEVER a crash and NEVER a + // silent exit-0 (was: fail-loud throw / exit 1); + // "infra" → benign infra, collector returns [] (exit 0). + // The invariant the corpus protects: an untrusted failure is never a green. // ------------------------------------------------------------------------- - describe("CLASS 1 fail-loud corpus", () => { + describe("CLASS 1 safe-outcome corpus", () => { const DRIFT_VALUE_WITH_STATUS_200 = formatDriftReport("OpenAI Chat (non-streaming text)", [ { path: "choices[0].message.content", @@ -652,37 +728,37 @@ describe("collectDriftEntries", () => { const rows: { name: string; messages: string[]; - throws: boolean; + outcome: "entry" | "quarantine" | "infra"; }[] = [ { - name: "a drift body containing the substring 'status 200' is surfaced, NOT swallowed as infra", + name: "a drift body containing the substring 'status 200' is surfaced as an entry, NOT swallowed as infra", messages: [DRIFT_VALUE_WITH_STATUS_200], - throws: false, // parsed into a structured entry + outcome: "entry", }, { - name: "a genuine drift report with a leading blank line is surfaced", + name: "a genuine drift report with a leading blank line is surfaced as an entry", messages: [GENUINE_DRIFT_WITH_STACK], - throws: false, + outcome: "entry", }, { - name: "an 'expected false to be true' hasGA shape is surfaced (canary), not swallowed", + name: "an 'expected false to be true' hasGA shape is surfaced as a (canary) entry, not swallowed", messages: [CANARY_NO_GA_MARKER], - throws: false, + outcome: "entry", }, { - name: "an 'expected […] to deeply equal []' canary shape is surfaced, not swallowed", + name: "an 'expected […] to deeply equal []' canary shape is surfaced as an entry, not swallowed", messages: [CANARY_MARKER_MULTI], - throws: false, + outcome: "entry", }, { - name: "a bare AssertionError with no infra token and no drift marker fails loud", + name: "a bare AssertionError with no infra token and no drift marker is quarantined (exit 5), not swallowed", messages: ["AssertionError: expected 1 to be 2 // Object.is equality\n at foo (x:1:1)"], - throws: true, + outcome: "quarantine", }, { - name: "an infra token confined to a stack frame fails loud (A3)", + name: "an infra token confined to a stack frame is quarantined (exit 5) (A3)", messages: [INFRA_TOKEN_IN_STACKFRAME_ONLY], - throws: true, + outcome: "quarantine", }, ]; @@ -698,16 +774,25 @@ describe("collectDriftEntries", () => { }), ), ); - if (row.throws) { - expect(() => collectDriftEntries(result)).toThrow(); + expect(() => collectDriftEntries(result)).not.toThrow(); + const { entries, quarantine } = collectDriftEntries(result); + if (row.outcome === "entry") { + expect(entries.length).toBeGreaterThan(0); + expect(exitCodeOf(result)).toBe(2); + } else if (row.outcome === "quarantine") { + expect(entries).toEqual([]); + expect(quarantine.length).toBeGreaterThan(0); + expect(exitCodeOf(result)).toBe(5); + expect(exitCodeOf(result)).not.toBe(0); } else { - expect(() => collectDriftEntries(result)).not.toThrow(); - expect(collectDriftEntries(result).length).toBeGreaterThan(0); + expect(entries).toEqual([]); + expect(quarantine).toEqual([]); + expect(exitCodeOf(result)).toBe(0); } }); } - it("still classifies genuine infra (body token) as benign — collector returns [] without throwing", () => { + it("still classifies genuine infra (body token) as benign — collector returns [] entries+quarantine (exit 0)", () => { const result = makeResult([ makeAssertion({ status: "failed", @@ -716,7 +801,127 @@ describe("collectDriftEntries", () => { failureMessages: [REAL_INFRA_BODY], }), ]); - expect(collectDriftEntries(result)).toEqual([]); + expect(entriesOf(result)).toEqual([]); + expect(quarantineOf(result)).toEqual([]); + expect(exitCodeOf(result)).toBe(0); + }); + }); + + // ------------------------------------------------------------------------- + // A1.4 TAXONOMY — the exit-code taxonomy end-to-end through the REAL collector + // + computeExitCode. Each row asserts the full path from a vitest failure + // shape to the process exit code main() would emit. + // ------------------------------------------------------------------------- + describe("exit-code taxonomy (collector → computeExitCode)", () => { + it("critical drift → exit 2", () => { + const driftText = formatDriftReport("OpenAI Chat (non-streaming text)", [SAMPLE_DIFF]); + const result = makeResult([ + makeAssertion({ + status: "failed", + ancestorTitles: ["OpenAI Chat Completions drift"], + failureMessages: [driftText], + }), + ]); + expect(entriesOf(result)).toHaveLength(1); + expect(quarantineOf(result)).toEqual([]); + expect(exitCodeOf(result)).toBe(2); + }); + + it("incident-5 unparseable failure → quarantine + exit 5 (NOT a throw)", () => { + const result = makeResult([ + makeAssertion({ + status: "failed", + ancestorTitles: ["Broken Suite"], + title: "a", + failureMessages: [ + "AssertionError: expected 1 to be 2 // Object.is equality\n at foo (/repo/src/__tests__/drift/x.drift.ts:8:13)", + ], + }), + ]); + expect(() => collectDriftEntries(result)).not.toThrow(); + expect(entriesOf(result)).toEqual([]); + expect(quarantineOf(result)).toHaveLength(1); + expect(exitCodeOf(result)).toBe(5); + }); + + it("all-infra failures → exit 0 (benign, collector returns nothing)", () => { + const result = makeResult([ + makeAssertion({ + status: "failed", + ancestorTitles: ["OpenAI Chat Completions drift"], + failureMessages: [REAL_INFRA_BODY], + }), + makeAssertion({ + status: "failed", + ancestorTitles: ["OpenAI Responses drift"], + failureMessages: [ + "INFRA_ERROR: upstream down\n at h (file:///repo/src/y.drift.ts:1:1)", + ], + }), + ]); + expect(entriesOf(result)).toEqual([]); + expect(quarantineOf(result)).toEqual([]); + expect(exitCodeOf(result)).toBe(0); + }); + + it("canary (unknown-model) failure → critical entry + exit 2", () => { + const result = makeResult([ + makeAssertion({ + status: "failed", + ancestorTitles: ["OpenAI Realtime API drift"], + title: "canary: GA realtime models available", + failureMessages: [CANARY_MARKER_MULTI], + }), + ]); + const entries = entriesOf(result); + expect(entries).toHaveLength(1); + expect(entries[0].diffs.every((d) => d.severity === "critical")).toBe(true); + expect(exitCodeOf(result)).toBe(2); + }); + + it("empty → fail-loud invariant: an all-unparseable batch is NEVER classified as a benign all-clear", () => { + // CLASS 1 root invariant surfaced at the collector: an empty evidence set + // (no positive infra evidence) must NOT be treated as infra. The batch is + // quarantined (exit 5), never a silent exit 0. + expect(classifyUnparseableAsInfra([])).toBe(false); + const result = makeResult([ + makeAssertion({ + status: "failed", + ancestorTitles: ["Broken Suite"], + title: "unrecognized", + failureMessages: [ + "AssertionError: expected 1 to be 2\n at foo (/repo/src/z.drift.ts:1:1)", + ], + }), + ]); + expect(exitCodeOf(result)).not.toBe(0); + expect(exitCodeOf(result)).toBe(5); + }); + + it("critical + quarantine together → exit 2 (critical dominates quarantine)", () => { + const driftText = formatDriftReport("OpenAI Chat (non-streaming text)", [SAMPLE_DIFF]); + const result = makeResult([ + makeAssertion({ + status: "failed", + ancestorTitles: ["OpenAI Chat Completions drift"], + title: "non-streaming text matches real API", + failureMessages: [driftText], + }), + // An unmapped-provider drift block → quarantined (never dropped) even + // though a real critical entry is also present. + makeAssertion({ + status: "failed", + ancestorTitles: ["UnknownProvider drift"], + title: "some scenario", + failureMessages: [formatDriftReport("UnknownProvider (streaming text)", [SAMPLE_DIFF])], + }), + ]); + const { entries, quarantine } = collectDriftEntries(result); + expect(entries).toHaveLength(1); + expect(entries[0].provider).toBe("OpenAI Chat"); + expect(quarantine).toHaveLength(1); + // Critical dominates: exit 2, not 5. + expect(exitCodeOf(result)).toBe(2); }); }); }); @@ -1020,6 +1225,45 @@ describe("classifyUnparseableAsInfra", () => { // the anchoring fix must not over-tighten and break genuine infra. expect(classifyUnparseableAsInfra([sample])).toBe(true); }); + + it(`[${source}] taxonomy (c): a bare "${sample}" reason → collector exit 0 (benign, no quarantine)`, () => { + // A1.4 extension: tie the infra classification to the exit-code taxonomy + // at the REAL collector surface. A bare infra-reason failure must be a + // benign exit 0 — NOT quarantined (exit 5) and NOT a crash. + const result = makeResult([ + makeAssertion({ + status: "failed", + ancestorTitles: ["OpenAI Chat Completions drift"], + title: "non-streaming text matches real API", + failureMessages: [sample], + }), + ]); + expect(() => collectDriftEntries(result)).not.toThrow(); + expect(entriesOf(result)).toEqual([]); + expect(quarantineOf(result)).toEqual([]); + expect(exitCodeOf(result)).toBe(0); + }); + + it(`[${source}] taxonomy (c'): a labelled "Real: ${sample}" drift value → NOT exit 0 (quarantined, exit 5)`, () => { + // Symmetric to (a) at the collector surface: a labelled body value that + // merely CONTAINS the infra phrase must never be swallowed as a green. + // It is not a full parseable drift block, so it is quarantined (exit 5). + const msg = + " Path: choices[0].message.content\n" + + " SDK: n/a\n" + + ` Real: ${sample}\n` + + " Mock: \n"; + const result = makeResult([ + makeAssertion({ + status: "failed", + ancestorTitles: ["OpenAI Chat Completions drift"], + title: "non-streaming text matches real API", + failureMessages: [msg], + }), + ]); + expect(exitCodeOf(result)).not.toBe(0); + expect(exitCodeOf(result)).toBe(5); + }); } }); }); From f021fe6d76f023811acb2c1cb0377b8e3c7af789 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 21:52:35 -0700 Subject: [PATCH 07/24] =?UTF-8?q?feat(drift):=20treat=20exit=205=20as=20qu?= =?UTF-8?q?arantine=20in=20drift-retry=20=E2=80=94=20no=20retry,=20propaga?= =?UTF-8?q?te=20immediately?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add EXIT_QUARANTINE=5 constant and quarantine?: boolean field on RetryResult. retryUntilStable now recognises exit 5 as a distinct terminal outcome: it does not retry (unlike exit 2) and marks quarantine:true on the result. The crash branch remains for all other non-{0,2,5} codes. main() already propagates the exit code via process.exit(result.exitCode) so exit 5 reaches the caller. Test: inject collector→5; RED (pre-change) quarantine field was undefined; GREEN asserts exitCode===5, quarantine===true, calls===1. --- scripts/drift-retry.ts | 18 +++++++++++++++++- src/__tests__/drift-retry.test.ts | 19 ++++++++++++++++++- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/scripts/drift-retry.ts b/scripts/drift-retry.ts index 6ba8e3a8..945bad5d 100644 --- a/scripts/drift-retry.ts +++ b/scripts/drift-retry.ts @@ -50,6 +50,7 @@ import { fileURLToPath } from "node:url"; // Collector exit-code contract (see drift-report-collector.ts header). export const EXIT_CLEAN = 0; export const EXIT_CRITICAL_DRIFT = 2; +export const EXIT_QUARANTINE = 5; // Defaults: keep the fleet of real-API calls small. 3 total attempts with a // ~45s backoff mirrors the observed transient window (the Fix Drift workflow @@ -78,7 +79,7 @@ export interface RetryOptions { } export interface RetryResult { - /** Final exit code to propagate (0 = clean/transient, 2 = persistent, other = crash). */ + /** Final exit code to propagate (0 = clean/transient, 2 = persistent, 5 = quarantine, other = crash). */ exitCode: number; /** True when at least one critical run was seen but a later run cleared it. */ transient: boolean; @@ -86,6 +87,8 @@ export interface RetryResult { criticalRuns: number; /** Per-attempt record, in order. */ attempts: RetryAttempt[]; + /** True when the collector exited with EXIT_QUARANTINE (5): unparseable output quarantined. */ + quarantine?: boolean; } /** @@ -125,6 +128,19 @@ export function retryUntilStable(opts: RetryOptions): RetryResult { continue; } + if (exitCode === EXIT_QUARANTINE) { + // Quarantine is a distinct terminal outcome: the collector encountered + // output it could not parse/classify. No retry — propagate immediately. + opts.log(`Collector exited ${exitCode} (quarantine) — propagating without retry.`); + return { + exitCode: EXIT_QUARANTINE, + transient: false, + criticalRuns, + attempts, + quarantine: true, + }; + } + // Any other code = collector crash / infra error. Do not retry — surface it. opts.log(`Collector exited ${exitCode} (not drift) — propagating without retry.`); return { exitCode, transient: false, criticalRuns, attempts }; diff --git a/src/__tests__/drift-retry.test.ts b/src/__tests__/drift-retry.test.ts index 373a0b23..9e34c13f 100644 --- a/src/__tests__/drift-retry.test.ts +++ b/src/__tests__/drift-retry.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from "vitest"; -import { retryUntilStable } from "../../scripts/drift-retry.js"; +import { EXIT_QUARANTINE, retryUntilStable } from "../../scripts/drift-retry.js"; import type { RetryAttempt, RetryOptions, RetryResult } from "../../scripts/drift-retry.js"; // --------------------------------------------------------------------------- @@ -137,4 +137,21 @@ describe("retryUntilStable", () => { expect(result.attempts.map((a: RetryAttempt) => a.exitCode)).toEqual([2, 2, 0]); }); + + it("treats exit 5 (quarantine) as a distinct terminal outcome — no retry, quarantine:true", () => { + // exit 5 = collector quarantined unparseable output; must propagate immediately + // without retrying and mark the result with quarantine:true. + const runner = fakeRunner([EXIT_QUARANTINE, 0, 0]); + const opts = makeOptions({ runCollector: runner.run }); + const result: RetryResult = retryUntilStable(opts); + + // Propagated as exit 5, not swallowed into the crash branch + expect(result.exitCode).toBe(EXIT_QUARANTINE); + // Quarantine flag set + expect(result.quarantine).toBe(true); + // No retry — only one attempt + expect(runner.calls()).toBe(1); + // Not treated as a transient drift event + expect(result.transient).toBe(false); + }); }); From 6ae96cdbe4f07ec08575377dc15ec0fb20f3488c Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 21:53:12 -0700 Subject: [PATCH 08/24] feat(drift): handle exit 5 (quarantine) as non-aborting in fix-drift workflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add elif branch for EXIT_CODE==5 in the collector step. Exit 5 means the collector quarantined unparseable output — it is not a script crash, so the workflow should continue rather than abort. Exit 1 (and all other non-{0,2,5} codes) remain fatal. The exit-2-only auto-fix gate (step 2 check) is unchanged. Red/green shell-harness: pre-change EXIT_CODE=5 routed to abort (fell into the non-0 catch-all); post-change routes to continue. --- .github/workflows/fix-drift.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/fix-drift.yml b/.github/workflows/fix-drift.yml index e5610bfd..a402bc16 100644 --- a/.github/workflows/fix-drift.yml +++ b/.github/workflows/fix-drift.yml @@ -63,6 +63,8 @@ jobs: echo "exit_code=$EXIT_CODE" >> $GITHUB_OUTPUT if [ "$EXIT_CODE" -eq 2 ]; then : # critical drift found, continue + elif [ "$EXIT_CODE" -eq 5 ]; then + : # quarantine: collector encountered unparseable output, continue without aborting elif [ "$EXIT_CODE" -ne 0 ]; then echo "::error::Collector script crashed with exit code $EXIT_CODE" exit $EXIT_CODE From 5286c72e180255f8704437594e1727c5b6c33404 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 21:53:47 -0700 Subject: [PATCH 09/24] test(drift): add per-provider model-family registry (B4.1) Add side-effect-free model-registry.ts exporting per-provider include/ exclude family Sets (seeded through normalizeModelFamily so membership is normalization-consistent and idempotent) plus a provider-agnostic NON_MODEL_TOKENS allowlist carrying gemini-interactions. include = text-generation families aimock mocks (derived from conformance tests/README/fixtures); exclude = retired/preview/non-text/voice families. The union is what the drift check (B4.2) subtracts against normalized live /models; leftover families are the drift signal. Unit test proves registry shape: include membership via normalized id, NON_MODEL_TOKENS carries gemini-interactions, no family on both include and exclude, and seeds are idempotent under normalization. --- src/__tests__/drift/model-registry.test.ts | 49 ++++++ src/__tests__/drift/model-registry.ts | 165 +++++++++++++++++++++ 2 files changed, 214 insertions(+) create mode 100644 src/__tests__/drift/model-registry.test.ts create mode 100644 src/__tests__/drift/model-registry.ts diff --git a/src/__tests__/drift/model-registry.test.ts b/src/__tests__/drift/model-registry.test.ts new file mode 100644 index 00000000..4bb63d24 --- /dev/null +++ b/src/__tests__/drift/model-registry.test.ts @@ -0,0 +1,49 @@ +/** + * Unit test for the per-provider model-family registry shape. + * + * This is the minimal shape/invariant probe for the registry itself. The full + * builder/fixture cross-check (every referenced model id resolves into the + * registry) is B4.3's job and lives in its own extended suite — this test only + * proves the registry is well-formed: + * - seeded families are normalization-consistent (membership works), + * - the provider-mode allowlist carries `gemini-interactions`, + * - no family appears in BOTH include and exclude for any provider. + */ +import { describe, it, expect } from "vitest"; +import { normalizeModelFamily } from "./model-family.js"; +import { includeFamilies, excludeFamilies, NON_MODEL_TOKENS } from "./model-registry.js"; + +describe("model-registry", () => { + it("include contains the normalized family of a mocked model id", () => { + expect(includeFamilies.gemini.has(normalizeModelFamily("gemini-2.5-flash", "gemini"))).toBe( + true, + ); + }); + + it("allowlists the gemini-interactions provider-mode token", () => { + expect(NON_MODEL_TOKENS.has("gemini-interactions")).toBe(true); + }); + + it("no family appears in both include and exclude for any provider", () => { + for (const provider of ["openai", "anthropic", "gemini"] as const) { + const inc = includeFamilies[provider]; + const exc = excludeFamilies[provider]; + const overlap = [...inc].filter((f) => exc.has(f)); + expect( + overlap, + `provider ${provider} has families on both lists: ${overlap.join(", ")}`, + ).toEqual([]); + } + }); + + it("seeds are idempotent under normalization (already family keys)", () => { + for (const provider of ["openai", "anthropic", "gemini"] as const) { + for (const family of includeFamilies[provider]) { + expect(normalizeModelFamily(family, provider)).toBe(family); + } + for (const family of excludeFamilies[provider]) { + expect(normalizeModelFamily(family, provider)).toBe(family); + } + } + }); +}); diff --git a/src/__tests__/drift/model-registry.ts b/src/__tests__/drift/model-registry.ts new file mode 100644 index 00000000..cd8444f2 --- /dev/null +++ b/src/__tests__/drift/model-registry.ts @@ -0,0 +1,165 @@ +/** + * Per-provider model-family REGISTRY for text-generation drift detection. + * + * Side-effect-free data module (no `describe`/`beforeAll`, like `voice-models.ts`) + * so both the live drift check (`models.drift.ts`) and its unit test can import + * the same seed data without transitively registering a drift suite. + * + * The drift check works by NORMALIZED FAMILY, not raw id: a provider's live + * `GET /models` list is normalized through `normalizeModelFamily(id, provider)` + * and each resulting family is subtracted against `include ∪ exclude`. Anything + * left over is an UNCLASSIFIED family — the drift signal. (The subtract/delta + * itself is `models.drift.ts`'s job; this module only owns the seed sets.) + * + * Two curated sets per provider, both keyed by the NORMALIZED family: + * + * - `include` — families aimock actually MOCKS. Derived from the model ids + * aimock's conformance tests, README, and fixtures reference (chat / text + * completion families). A live family that normalizes into this set is known + * and generates no drift. Seeds are already family keys, so dated snapshots + * of an included family (`gpt-4o-2024-08-06` → `gpt-4o`) collapse onto them. + * + * - `exclude` — families we deliberately DO NOT treat as text-generation drift: + * retired/legacy ids, preview-only ids, and non-text families (embeddings, + * image, tts/audio/transcribe voice families — the last are the realtime + * canary's responsibility in `voice-models.ts`, not this text check). A live + * family in this set is expected and generates no drift. + * + * A family MUST NOT appear in both `include` and `exclude` for the same provider + * (asserted in the unit test) — the two sets partition the "already classified" + * space; their union is what the drift check subtracts. + * + * `NON_MODEL_TOKENS` is a provider-agnostic allowlist of aimock "provider mode" + * names — internal routing names that reuse a real upstream API key but are NOT + * model ids any provider's `/models` endpoint exposes (e.g. `gemini-interactions` + * reuses the Gemini upstream key). The README documents them, so a greedy source + * scrape or a builder cross-check would otherwise treat them as unknown model + * ids and flag guaranteed false positives. They live here so both the drift + * check and the builder cross-check (B4.3) exclude the exact same tokens. + * + * Every set is built THROUGH `normalizeModelFamily` so membership tests are + * normalization-consistent and the seeds are provably idempotent (a seed carrying + * a stray dated/build suffix would silently normalize to a different key — the + * `.map(normalize)` makes that impossible). + */ + +import { normalizeModelFamily } from "./model-family.js"; + +type Provider = "openai" | "anthropic" | "gemini"; + +/** Build a family Set for a provider, seeding each entry through the normalizer. */ +function familySet(provider: Provider, families: string[]): Set { + return new Set(families.map((f) => normalizeModelFamily(f, provider))); +} + +/** + * Families aimock MOCKS, per provider. These are the canonical text-generation + * family keys referenced by aimock's conformance tests, README, and fixtures. + * Dated/versioned variants normalize onto these (e.g. `gpt-4o-2024-08-06` → + * `gpt-4o`, `claude-3-5-sonnet-20241022` → `claude-3-5-sonnet`). + */ +export const includeFamilies: Record> = { + openai: familySet("openai", [ + // Chat / completion families + "gpt-3.5-turbo", + "gpt-4", + "gpt-4-turbo", + "gpt-4o", + "gpt-4o-mini", + "gpt-4.1", + "gpt-4.1-mini", + "gpt-4.1-nano", + "gpt-5", + "gpt-5-mini", + ]), + anthropic: familySet("anthropic", [ + // Claude 3 / 3.5 / 3.7 families + "claude-3-opus", + "claude-3-sonnet", + "claude-3-haiku", + "claude-3-5-sonnet", + "claude-3-5-haiku", + "claude-3-7-sonnet", + // Claude 4 families + "claude-opus-4", + "claude-sonnet-4", + "claude-haiku-4", + ]), + gemini: familySet("gemini", [ + // Gemini 1.5 / 2.0 / 2.5 text families + "gemini-1.5-pro", + "gemini-1.5-flash", + "gemini-2.0-flash", + "gemini-2.5-flash", + "gemini-2.5-pro", + ]), +}; + +/** + * Families we deliberately DO NOT count as text-generation drift, per provider: + * retired/legacy ids, preview-only ids, and non-text families (embeddings, + * image, and the voice/audio/tts/transcribe families the realtime canary in + * `voice-models.ts` owns). A live family here is expected, not drift. + */ +export const excludeFamilies: Record> = { + openai: familySet("openai", [ + // Retired / legacy chat + "gpt-3", + "gpt-3.5", + // Embeddings (non-text-generation) + "text-embedding-3-small", + "text-embedding-3-large", + "text-embedding-ada-002", + // Image models + "dall-e-2", + "dall-e-3", + "gpt-image-1", + // Voice / audio / tts / transcribe — owned by the realtime canary + "tts-1", + "tts-1-hd", + "whisper-1", + "gpt-audio", + "gpt-audio-mini", + "gpt-audio-1.5", + "gpt-4o-mini-tts", + "gpt-4o-transcribe", + "gpt-4o-mini-transcribe", + "gpt-4o-transcribe-diarize", + "gpt-realtime", + "gpt-realtime-mini", + "gpt-realtime-2", + "gpt-realtime-2.1", + "gpt-realtime-2.1-mini", + "gpt-realtime-1.5", + "gpt-realtime-translate", + "gpt-realtime-whisper", + // Preview-only realtime + "gpt-4o-realtime-preview", + "gpt-4o-mini-realtime-preview", + ]), + anthropic: familySet("anthropic", [ + // Retired / legacy Claude ids + "claude-v3", + "claude-2", + "claude-instant-1", + ]), + gemini: familySet("gemini", [ + // Retired / legacy + "gemini-pro", + // Embeddings (non-text-generation) + "text-embedding-004", + // Experimental / preview / thinking-preview + "gemini-2.0-flash-exp", + "gemini-2.0-flash-thinking-exp", + // Live/full-duplex voice — owned by the realtime canary, not this text check + "gemini-live", + ]), +}; + +/** + * aimock "provider mode" names: internal routing names that reuse a real + * upstream provider key but are NOT model ids any provider's `/models` endpoint + * exposes. Provider-agnostic allowlist so both the drift check and the builder + * cross-check exclude the exact same tokens (never false-positive drift). + */ +export const NON_MODEL_TOKENS: Set = new Set(["gemini-interactions"]); From 210720f8e81a201479077a60b5ad22bb73ab8997 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 21:53:53 -0700 Subject: [PATCH 10/24] feat(drift): emit quarantine ::error:: label for exit 5 in test-drift workflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add elif branch for EXIT_CODE==5 in the collector step of the drift job. Exit 5 emits a distinct quarantine-labelled ::error:: annotation ("quarantined unparseable output — manual triage required") and re-exits 5 via the existing exit "$EXIT_CODE" path (O1). The generic crash message is reserved for all other non-{0,2,5} codes. The exit_code==2 fail step is untouched. Red/green shell-harness: pre-change EXIT_CODE=5 emitted the generic "crashed" message; post-change emits the quarantine label and still exits 5. --- .github/workflows/test-drift.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/test-drift.yml b/.github/workflows/test-drift.yml index 18b099be..4aed033e 100644 --- a/.github/workflows/test-drift.yml +++ b/.github/workflows/test-drift.yml @@ -76,6 +76,9 @@ jobs: echo "exit_code=$EXIT_CODE" >> "$GITHUB_OUTPUT" if [ "$EXIT_CODE" -eq 2 ]; then : # critical drift persisted across retries, continue + elif [ "$EXIT_CODE" -eq 5 ]; then + echo "::error::Drift collector quarantined unparseable output (exit 5) — manual triage required" + exit "$EXIT_CODE" elif [ "$EXIT_CODE" -ne 0 ]; then echo "::error::Collector script crashed with exit code $EXIT_CODE" exit "$EXIT_CODE" From b8e404d0a8bc6aa37b4846a732f38a5dc7725c8d Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 22:01:13 -0700 Subject: [PATCH 11/24] test(drift): rewrite models.drift to family normalize+subtract (B4.2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Delete the prose-scrape path (scrapeModels, sourceFiles, GEMINI_MODEL_PATTERN, filterStableGeminiModels, AIMOCK_GEMINI_PROVIDER_MODES) that caused incident 5. Each provider block now fetches live /models, normalizes every id to its family via normalizeModelFamily, subtracts includeFamilies ∪ excludeFamilies and NON_MODEL_TOKENS, and surfaces the unclassified remainder as a formatDriftReport-wrapped failing assertion so the collector routes it (exit-2 auto-fix / exit-5 quarantine). Adds a no-key regression suite exercising the real pipeline: incident-2 dated snapshots of known families produce zero delta; a prose provider-mode token can never enter; a genuinely new family still fires. --- src/__tests__/drift/models.drift.ts | 236 +++++++++++++++++----------- 1 file changed, 140 insertions(+), 96 deletions(-) diff --git a/src/__tests__/drift/models.drift.ts b/src/__tests__/drift/models.drift.ts index 1e5fa556..053e2cb5 100644 --- a/src/__tests__/drift/models.drift.ts +++ b/src/__tests__/drift/models.drift.ts @@ -1,88 +1,146 @@ /** - * Model deprecation checks — verify that models referenced in aimock's - * tests, docs, and examples still exist at each provider. + * Model-family drift check — verify that each provider's LIVE `GET /models` + * list contains no UNCLASSIFIED text-generation family. + * + * How it works (no source scraping — that path caused incident 5): + * 1. Fetch the provider's live model list (`list*Models`). + * 2. Normalize every id to its FAMILY key (`normalizeModelFamily`), so dated + * snapshots and build tags collapse onto their family + * (`gpt-4o-2024-08-06` → `gpt-4o`, `tts-1-1106` → `tts-1`). + * 3. Subtract the already-classified space: `includeFamilies[provider] ∪ + * excludeFamilies[provider]`, and drop the provider-agnostic + * `NON_MODEL_TOKENS` allowlist. + * 4. Whatever remains is an UNCLASSIFIED family — the drift signal. It is + * surfaced as a FAILING assertion wrapped in a `formatDriftReport` block so + * the collector (`scripts/drift-report-collector.ts`) routes it: the + * `API DRIFT DETECTED` block parses into critical entries (exit-2 auto-fix + * lane); anything it cannot map falls to exit-5 quarantine. + * + * Comparing NORMALIZED families (not raw ids) is what makes this converge: + * appending every new dated snapshot to a known-id set never stabilizes and + * turns the daily job permanently red on false positives (incident 2). Only a + * genuinely NEW family (e.g. `gpt-live`) is ever flagged. + * + * Because nothing is scraped from source, an aimock "provider mode" prose token + * (e.g. `gemini-interactions`) can never enter the pipeline as a candidate id — + * the only inputs are the provider's own `/models` payload. */ import { describe, it, expect } from "vitest"; -import * as fs from "node:fs"; -import * as path from "node:path"; import { listOpenAIModels, listAnthropicModels, listGeminiModels } from "./providers.js"; +import { normalizeModelFamily } from "./model-family.js"; +import { includeFamilies, excludeFamilies, NON_MODEL_TOKENS } from "./model-registry.js"; +import { formatDriftReport } from "./schema.js"; -// --------------------------------------------------------------------------- -// Scrape referenced models from the codebase -// --------------------------------------------------------------------------- +type Provider = "openai" | "anthropic" | "gemini"; -const PROJECT_ROOT = path.resolve(import.meta.dirname, "..", "..", ".."); - -export function scrapeModels(pattern: RegExp, files: string[]): string[] { - const models = new Set(); - for (const file of files) { - const filePath = path.join(PROJECT_ROOT, file); - if (!fs.existsSync(filePath)) continue; - const content = fs.readFileSync(filePath, "utf-8"); - pattern.lastIndex = 0; - let match; - while ((match = pattern.exec(content)) !== null) { - models.add(match[1]); - } +/** + * Reduce a live `/models` list to the UNCLASSIFIED families: normalize each id, + * then drop everything already in `include ∪ exclude` or on the non-model + * allowlist. The returned list (sorted, de-duplicated) is the drift signal. + * + * Exported so the co-located regression suite can exercise the EXACT + * enumerate→normalize→subtract pipeline the live check relies on, with an + * injected payload — no reimplementation. + */ +export function unclassifiedFamilies(modelIds: string[], provider: Provider): string[] { + const known = new Set([...includeFamilies[provider], ...excludeFamilies[provider]]); + const unclassified = new Set(); + for (const id of modelIds) { + const family = normalizeModelFamily(id, provider); + if (known.has(family)) continue; + if (NON_MODEL_TOKENS.has(family) || NON_MODEL_TOKENS.has(id)) continue; + unclassified.add(family); } - return [...models]; + return [...unclassified].sort(); } -export const sourceFiles = [ - "src/__tests__/api-conformance.test.ts", - "src/__tests__/ws-api-conformance.test.ts", - "README.md", - "fixtures/example-greeting.json", - "fixtures/example-multi-turn.json", - "fixtures/example-tool-call.json", -]; +/** + * Assert that a live `/models` list has zero unclassified families. On failure, + * emit one critical drift diff per unclassified family inside a + * `formatDriftReport` block so the collector routes it to the exit-2 auto-fix + * lane (provider names match `PROVIDER_MAP` keys in the collector). + */ +function assertNoUnclassifiedFamilies( + modelIds: string[], + provider: Provider, + context: string, +): void { + const unclassified = unclassifiedFamilies(modelIds, provider); + const report = + unclassified.length > 0 + ? formatDriftReport( + context, + unclassified.map((family) => ({ + path: `models/${family}`, + severity: "critical" as const, + issue: + `Unclassified model family "${family}" in ${provider} /models — ` + + `add it to includeFamilies (aimock mocks it) or excludeFamilies ` + + `(non-text / retired / preview) in model-registry.ts`, + expected: "(family in includeFamilies ∪ excludeFamilies)", + real: family, + mock: "", + })), + ) + : `No drift detected: ${context}`; + expect(unclassified, report).toEqual([]); +} -// Regex used to scrape Gemini model ids from the source files above. Greedy -// on purpose so we catch versioned/dated ids (e.g. gemini-2.5-flash), but that -// greed also grabs any `gemini-*` token appearing in prose — see the stable -// filter below for what gets excluded. -export const GEMINI_MODEL_PATTERN = /\b(gemini-(?:[\w.-]+))\b/g; +// --------------------------------------------------------------------------- +// Regression suite (no live keys) — exercises the REAL pipeline with injected +// `/models` payloads. Runs unconditionally so the drift job proves the +// enumerate→normalize→subtract behavior even when live keys are absent. +// --------------------------------------------------------------------------- -// aimock exposes "provider modes" — internal names that route to a real -// upstream API but are NOT themselves model ids exposed by that provider. The -// README documents them (e.g. `gemini-interactions` reuses the Gemini upstream -// key), so the greedy scraper above grabs them as if they were Gemini models. -// They will never appear in Google's model list, so checking them for drift is -// a guaranteed false positive. Exclude them explicitly. -const AIMOCK_GEMINI_PROVIDER_MODES = new Set(["gemini-interactions"]); +describe("model-family pipeline (injected /models)", () => { + it("incident 2: dated snapshots of included families produce ZERO drift", () => { + // Payload of dated/build-tag snapshots whose FAMILIES are all in + // includeFamilies/excludeFamilies. The old scrape+substring path would have + // flagged these dated ids as unknown; the normalize+subtract path collapses + // each onto its known family, so the unclassified delta must be empty. + const openaiPayload = [ + "gpt-4o-2024-08-06", // → gpt-4o (include) + "gpt-4o-mini-2024-07-18", // → gpt-4o-mini (include) + "gpt-4.1-2025-04-14", // → gpt-4.1 (include) + "gpt-audio-2025-08-28", // → gpt-audio (exclude) + "tts-1-1106", // → tts-1 (exclude) + "gpt-4o-mini-tts-2025-12-15", // → gpt-4o-mini-tts (exclude) + ]; + expect(unclassifiedFamilies(openaiPayload, "openai")).toEqual([]); + + // Gemini dated variants collapse via the same dated-snapshot strip. + const geminiPayload = [ + "gemini-2.5-flash", // include + "gemini-2.0-flash", // include + "gemini-1.5-pro-2024-05-14", // → gemini-1.5-pro (include) + ]; + expect(unclassifiedFamilies(geminiPayload, "gemini")).toEqual([]); + }); -// Narrow a raw scrape of `gemini-*` tokens down to real, checkable model ids by -// dropping (a) experimental/live/preview ids, (b) markdown anchor-link -// fragments, and (c) aimock provider-mode names that are documentation prose, -// not provider model ids. Exported so the regression suite can exercise the -// exact filtering the drift check relies on. -export function filterStableGeminiModels(referenced: string[]): string[] { - return referenced.filter( - (m) => - !m.includes("-exp") && - !m.includes("-live") && - !m.includes("bidigeneratecontent") && - !AIMOCK_GEMINI_PROVIDER_MODES.has(m), - ); -} + it("a prose provider-mode token can never enter as a candidate", () => { + // Nothing is scraped from source, so a `gemini-interactions`-style token can + // only appear if a provider's own /models returned it — and even then it is + // on NON_MODEL_TOKENS and never becomes drift. + expect(unclassifiedFamilies(["gemini-interactions"], "gemini")).toEqual([]); + }); + + it("a genuinely new family IS flagged as unclassified drift", () => { + // Guard the other side: the canary must still fire for a real new family. + expect(unclassifiedFamilies(["gpt-live"], "openai")).toEqual(["gpt-live"]); + // Single-digit trailing suffix is NOT a build tag, so it stays unknown. + expect(unclassifiedFamilies(["gpt-live-1"], "openai")).toEqual(["gpt-live-1"]); + }); +}); // --------------------------------------------------------------------------- // OpenAI // --------------------------------------------------------------------------- -describe.skipIf(!process.env.OPENAI_API_KEY)("OpenAI model availability", () => { - it("models used in aimock tests are still available", async () => { +describe.skipIf(!process.env.OPENAI_API_KEY)("OpenAI Chat model-family availability", () => { + it("live /models contains no unclassified family", async () => { const models = await listOpenAIModels(process.env.OPENAI_API_KEY!); - const referenced = scrapeModels(/\b(gpt-4o(?:-mini)?|gpt-4|gpt-3\.5-turbo)\b/g, sourceFiles); - - if (referenced.length === 0) return; // no models found to check - - for (const m of referenced) { - // OpenAI model list may include versioned variants — check prefix match - const found = models.some((available) => available === m || available.startsWith(`${m}-`)); - expect(found, `Model ${m} no longer available at OpenAI`).toBe(true); - } + assertNoUnclassifiedFamilies(models, "openai", "OpenAI Chat (live /models family canary)"); }); }); @@ -90,41 +148,27 @@ describe.skipIf(!process.env.OPENAI_API_KEY)("OpenAI model availability", () => // Anthropic // --------------------------------------------------------------------------- -describe.skipIf(!process.env.ANTHROPIC_API_KEY)("Anthropic model availability", () => { - it("models used in aimock tests are still available", async () => { - const models = await listAnthropicModels(process.env.ANTHROPIC_API_KEY!); - const referenced = scrapeModels( - /\b(claude-3(?:\.\d+)?-(?:opus|sonnet|haiku)(?:-\d{8})?)\b/g, - sourceFiles, - ); - - if (referenced.length === 0) return; - - for (const m of referenced) { - const found = models.some((available) => available === m || available.startsWith(m)); - expect(found, `Model ${m} no longer available at Anthropic`).toBe(true); - } - }); -}); +describe.skipIf(!process.env.ANTHROPIC_API_KEY)( + "Anthropic Claude model-family availability", + () => { + it("live /models contains no unclassified family", async () => { + const models = await listAnthropicModels(process.env.ANTHROPIC_API_KEY!); + assertNoUnclassifiedFamilies( + models, + "anthropic", + "Anthropic Claude (live /models family canary)", + ); + }); + }, +); // --------------------------------------------------------------------------- // Gemini // --------------------------------------------------------------------------- -describe.skipIf(!process.env.GOOGLE_API_KEY)("Gemini model availability", () => { - it("models used in aimock tests are still available", async () => { +describe.skipIf(!process.env.GOOGLE_API_KEY)("Google Gemini model-family availability", () => { + it("live /models contains no unclassified family", async () => { const models = await listGeminiModels(process.env.GOOGLE_API_KEY!); - const referenced = scrapeModels(GEMINI_MODEL_PATTERN, sourceFiles); - - if (referenced.length === 0) return; - - // Drop experimental/live ids, markdown anchor fragments, and aimock - // provider-mode names (see filterStableGeminiModels). - const stable = filterStableGeminiModels(referenced); - - for (const m of stable) { - const found = models.some((available) => available === m || available.startsWith(m)); - expect(found, `Model ${m} no longer available at Gemini`).toBe(true); - } + assertNoUnclassifiedFamilies(models, "gemini", "Google Gemini (live /models family canary)"); }); }); From 3f39c8c63b016b7f69a97ef9ec5115ac350a596b Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 22:05:07 -0700 Subject: [PATCH 12/24] fix(drift): strip Anthropic contiguous -YYYYMMDD snapshot in normalizeModelFamily Anthropic dates its model ids with an undelimited -YYYYMMDD suffix (e.g. claude-3-5-sonnet-20241022) rather than the dashed -YYYY-MM-DD form the shared core strips. Add an anthropic-only /-\d{8}$/ strip via the existing provider hook, applied inside the same idempotent reduction loop, so dated Claude ids collapse onto their family. openai/gemini keep only the shared core so a non-date 8-digit tail is never over-stripped. --- src/__tests__/drift/model-family.test.ts | 16 ++++++++++++ src/__tests__/drift/model-family.ts | 31 ++++++++++++++++++------ 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/src/__tests__/drift/model-family.test.ts b/src/__tests__/drift/model-family.test.ts index de4ce61c..da230e95 100644 --- a/src/__tests__/drift/model-family.test.ts +++ b/src/__tests__/drift/model-family.test.ts @@ -16,4 +16,20 @@ describe("normalizeModelFamily", () => { it("does not strip a single-digit suffix", () => { expect(normalizeModelFamily("gpt-live-1", "openai")).toBe("gpt-live-1"); }); + + it("strips a trailing Anthropic contiguous 8-digit snapshot for anthropic", () => { + expect(normalizeModelFamily("claude-3-5-sonnet-20241022", "anthropic")).toBe( + "claude-3-5-sonnet", + ); + expect(normalizeModelFamily("claude-3-7-sonnet-20250219", "anthropic")).toBe( + "claude-3-7-sonnet", + ); + }); + + it("does NOT strip a contiguous 8-digit tail for openai/gemini", () => { + // The 8-digit rule is Anthropic-only; a non-date 8-digit tail on another + // provider must survive so it is not silently over-stripped. + expect(normalizeModelFamily("gpt-weird-12345678", "openai")).toBe("gpt-weird-12345678"); + expect(normalizeModelFamily("gemini-weird-12345678", "gemini")).toBe("gemini-weird-12345678"); + }); }); diff --git a/src/__tests__/drift/model-family.ts b/src/__tests__/drift/model-family.ts index 75b9c17e..6489f64a 100644 --- a/src/__tests__/drift/model-family.ts +++ b/src/__tests__/drift/model-family.ts @@ -10,7 +10,8 @@ * red on false positives. Comparing the NORMALIZED family instead means only a * genuinely new family (e.g. `gpt-live`) is ever flagged. * - * Two suffix shapes are stripped, repeatedly, from the END of the id: + * Two suffix shapes are stripped, repeatedly, from the END of the id for ALL + * providers (the SHARED CORE): * - a dated snapshot `-YYYY-MM-DD` (e.g. `-2025-08-28`) * - a build/version tag `-NNN` or `-NNNN` (3–4 digits, e.g. `-1106`) * @@ -20,23 +21,37 @@ * NOT stripped, so `gpt-live-1` normalizes to `gpt-live-1` — an unknown family — * and stays flagged (the whole point of the canary). * - * The `provider` argument is a forward hook: the dated-snapshot/build-tag strip - * below is the SHARED CORE applied identically for all three providers today, so - * `normalizeModelFamily(id, "openai")` is byte-identical to the historical - * `normalizeVoiceModelFamily(id)`. Provider-specific normalization can branch off - * `provider` later without touching call sites. + * The `provider` argument selects a per-provider EXTRA rule on top of the shared + * core: + * - `anthropic` additionally strips a CONTIGUOUS 8-digit snapshot `-YYYYMMDD` + * (e.g. `claude-3-5-sonnet-20241022` → `claude-3-5-sonnet`). Anthropic dates + * its ids with an undelimited `-YYYYMMDD` suffix rather than the dashed + * `-YYYY-MM-DD` form, so without this every dated Claude id would + * false-positive as drift (the incident-2 class, for Anthropic). + * - `openai` / `gemini` keep ONLY the shared core — the 8-digit strip is NOT + * applied to them, so a non-date 8-digit tail is never over-stripped, and + * `normalizeModelFamily(id, "openai")` stays byte-identical to the historical + * `normalizeVoiceModelFamily(id)`. + * + * The per-provider rule runs inside the SAME reduction loop as the shared core, + * so it stays idempotent: a dated snapshot following a build tag still fully + * reduces. */ const DATED_SNAPSHOT_SUFFIX = /-\d{4}-\d{2}-\d{2}$/; const BUILD_TAG_SUFFIX = /-\d{3,4}$/; +/** Anthropic contiguous `-YYYYMMDD` snapshot suffix (undelimited date). */ +const ANTHROPIC_DATE_SUFFIX = /-\d{8}$/; export function normalizeModelFamily( id: string, provider: "openai" | "anthropic" | "gemini", ): string { - void provider; let family = id; for (;;) { - const stripped = family.replace(DATED_SNAPSHOT_SUFFIX, "").replace(BUILD_TAG_SUFFIX, ""); + let stripped = family.replace(DATED_SNAPSHOT_SUFFIX, "").replace(BUILD_TAG_SUFFIX, ""); + if (provider === "anthropic") { + stripped = stripped.replace(ANTHROPIC_DATE_SUFFIX, ""); + } if (stripped === family) break; family = stripped; } From 1ee0a827fb6c79a8c7b904a25f5bdb95370c3ee1 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 22:05:15 -0700 Subject: [PATCH 13/24] test(drift): restore Anthropic dated-id incident-2 regression case Re-add the Anthropic dated-id case B4.2 removed to the incident-2 injected-payload regression: claude-3-5-sonnet-20241022, claude-3-7-sonnet-20250219, claude-3-5-haiku-20241022 now normalize onto their included families and assert ZERO unclassified delta, guarding the incident-2 class for Anthropic's contiguous -YYYYMMDD snapshots. --- src/__tests__/drift/models.drift.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/__tests__/drift/models.drift.ts b/src/__tests__/drift/models.drift.ts index 053e2cb5..d2737a37 100644 --- a/src/__tests__/drift/models.drift.ts +++ b/src/__tests__/drift/models.drift.ts @@ -116,6 +116,17 @@ describe("model-family pipeline (injected /models)", () => { "gemini-1.5-pro-2024-05-14", // → gemini-1.5-pro (include) ]; expect(unclassifiedFamilies(geminiPayload, "gemini")).toEqual([]); + + // Anthropic uses a CONTIGUOUS 8-digit snapshot suffix (`-YYYYMMDD`), not the + // dashed `-YYYY-MM-DD` form. These must collapse onto their included family + // via the anthropic-specific strip, or every dated Claude id false-positives + // as drift (the incident-2 class, for Anthropic). + const anthropicPayload = [ + "claude-3-5-sonnet-20241022", // → claude-3-5-sonnet (include) + "claude-3-7-sonnet-20250219", // → claude-3-7-sonnet (include) + "claude-3-5-haiku-20241022", // → claude-3-5-haiku (include) + ]; + expect(unclassifiedFamilies(anthropicPayload, "anthropic")).toEqual([]); }); it("a prose provider-mode token can never enter as a candidate", () => { From d43e363a289afb0696af902822d3f25d3daf478c Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 22:12:19 -0700 Subject: [PATCH 14/24] =?UTF-8?q?test(drift):=20add=20=C2=A76.1c=20builder?= =?UTF-8?q?/fixture=20cross-check,=20delete=20models-scrape=20regression?= =?UTF-8?q?=20suite=20(B4.3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend model-registry.test.ts with the §6.1c cross-check (Part 2): a static table of model ids referenced by aimock's DEFAULT_MODELS (server.ts), conformance tests, and fixture-blocks tests, organised by provider. Every entry must resolve via normalizeModelFamily to a family in includeFamilies (aimock mocks it) or excludeFamilies (retired/non-text/preview). The gemini-interactions provider-mode token must appear on NON_MODEL_TOKENS. A stray builder id or dropped allowlist entry fails the unit test before it can cause a live crash in the drift job. Delete models-scrape.drift.ts: its two tests exercised scrapeModels, filterStableGeminiModels, and sourceFiles which were deleted in B4.2 (they tested the now-gone scrape path). Both tests were already failing with TypeError. grep -rn "models-scrape" src/ confirms no remaining importers. --- src/__tests__/drift/model-registry.test.ts | 150 ++++++++++++++++++++- src/__tests__/drift/models-scrape.drift.ts | 48 ------- 2 files changed, 145 insertions(+), 53 deletions(-) delete mode 100644 src/__tests__/drift/models-scrape.drift.ts diff --git a/src/__tests__/drift/model-registry.test.ts b/src/__tests__/drift/model-registry.test.ts index 4bb63d24..b6e02703 100644 --- a/src/__tests__/drift/model-registry.test.ts +++ b/src/__tests__/drift/model-registry.test.ts @@ -1,18 +1,35 @@ /** - * Unit test for the per-provider model-family registry shape. + * Unit test for the per-provider model-family registry shape, plus the §6.1c + * builder/fixture cross-check. * - * This is the minimal shape/invariant probe for the registry itself. The full - * builder/fixture cross-check (every referenced model id resolves into the - * registry) is B4.3's job and lives in its own extended suite — this test only - * proves the registry is well-formed: + * Part 1 (registry shape): the minimal invariant probe for model-registry.ts: * - seeded families are normalization-consistent (membership works), * - the provider-mode allowlist carries `gemini-interactions`, * - no family appears in BOTH include and exclude for any provider. + * + * Part 2 (builder/fixture cross-check — §6.1c): every model id that aimock's + * builders and fixtures reference must resolve, via normalizeModelFamily, to a + * family that is either in includeFamilies for its provider (aimock mocks it) + * or in excludeFamilies (retired/non-text/preview). Non-model "provider mode" + * tokens (e.g. `gemini-interactions`) must appear on NON_MODEL_TOKENS. A stray + * builder model id or a misclassified prose token must fail this test — never a + * live crash in the drift job. + * + * The cross-check table is derived from: + * - `DEFAULT_MODELS` in src/server.ts (aimock's advertised /v1/models list) + * - Dated/versioned ids used in conformance and fixture-blocks tests + * - The `gemini-interactions` provider-mode token documented in README + * + * The table is intentionally static (no source scraping) so it exercises the + * EXACT normalizeModelFamily + registry membership path the live drift check + * relies on, with no live-key dependency. */ import { describe, it, expect } from "vitest"; import { normalizeModelFamily } from "./model-family.js"; import { includeFamilies, excludeFamilies, NON_MODEL_TOKENS } from "./model-registry.js"; +// ─── Part 1: registry shape invariants ─────────────────────────────────────── + describe("model-registry", () => { it("include contains the normalized family of a mocked model id", () => { expect(includeFamilies.gemini.has(normalizeModelFamily("gemini-2.5-flash", "gemini"))).toBe( @@ -47,3 +64,126 @@ describe("model-registry", () => { } }); }); + +// ─── Part 2: §6.1c builder/fixture cross-check ─────────────────────────────── + +type Provider = "openai" | "anthropic" | "gemini"; + +/** + * The static cross-check table. Each entry is a model id that aimock's + * builders, fixtures, or DEFAULT_MODELS reference, tagged with its provider. + * Every entry MUST normalize to a family in includeFamilies[provider] or + * excludeFamilies[provider]. Non-model tokens go in NON_MODEL_TOKENS_UNDER_TEST. + * + * Sources: + * - DEFAULT_MODELS (src/server.ts): the ids aimock advertises at /v1/models + * - Conformance / fixture-blocks tests: dated snapshots, family aliases + * - gemini-interactions: provider-mode token (NON_MODEL_TOKENS, not a real id) + */ +const BUILDER_FIXTURE_MODEL_IDS: Array<{ id: string; provider: Provider }> = [ + // ── DEFAULT_MODELS (src/server.ts) ───────────────────────────────────────── + { id: "gpt-4", provider: "openai" }, + { id: "gpt-4o", provider: "openai" }, + { id: "claude-3-5-sonnet-20241022", provider: "anthropic" }, // dated snapshot + { id: "gemini-2.0-flash", provider: "gemini" }, + // text-embedding-3-small is in excludeFamilies (non-text-generation) — see below + + // ── OpenAI: additional families referenced in conformance/fixture tests ──── + { id: "gpt-3.5-turbo", provider: "openai" }, + { id: "gpt-4-turbo", provider: "openai" }, + { id: "gpt-4-turbo-2024-04-09", provider: "openai" }, // dated → gpt-4-turbo + { id: "gpt-4o-2024-08-06", provider: "openai" }, // dated → gpt-4o + { id: "gpt-4o-mini", provider: "openai" }, + { id: "gpt-4.1", provider: "openai" }, + { id: "gpt-4.1-mini", provider: "openai" }, + { id: "gpt-4.1-nano", provider: "openai" }, + { id: "gpt-5", provider: "openai" }, + { id: "gpt-5-mini", provider: "openai" }, + + // ── OpenAI: excluded families (embeddings, image, voice/audio) ──────────── + { id: "text-embedding-3-small", provider: "openai" }, // exclude: embeddings + { id: "gpt-image-1", provider: "openai" }, // exclude: image + { id: "gpt-audio", provider: "openai" }, // exclude: voice canary + { id: "gpt-audio-mini", provider: "openai" }, // exclude: voice canary + { id: "gpt-audio-2025-08-28", provider: "openai" }, // dated → gpt-audio (exclude) + { id: "tts-1", provider: "openai" }, // exclude: tts + { id: "gpt-4o-mini-tts", provider: "openai" }, // exclude: tts + { id: "gpt-4o-mini-tts-2025-12-15", provider: "openai" }, // dated → gpt-4o-mini-tts (exclude) + { id: "gpt-4o-transcribe", provider: "openai" }, // exclude: transcribe + { id: "gpt-4o-mini-transcribe", provider: "openai" }, // exclude: transcribe + { id: "gpt-realtime", provider: "openai" }, // exclude: realtime canary + { id: "gpt-realtime-mini", provider: "openai" }, // exclude: realtime canary + { id: "gpt-4o-realtime-preview", provider: "openai" }, // exclude: preview realtime + { id: "gpt-4o-mini-realtime-preview", provider: "openai" }, // exclude: preview realtime + + // ── Anthropic ───────────────────────────────────────────────────────────── + { id: "claude-3-opus", provider: "anthropic" }, + { id: "claude-3-opus-20240229", provider: "anthropic" }, // dated → claude-3-opus + { id: "claude-3-sonnet", provider: "anthropic" }, + { id: "claude-3-haiku", provider: "anthropic" }, + { id: "claude-3-5-sonnet", provider: "anthropic" }, + { id: "claude-3-5-haiku", provider: "anthropic" }, + { id: "claude-3-5-haiku-20241022", provider: "anthropic" }, // dated → claude-3-5-haiku + { id: "claude-3-7-sonnet", provider: "anthropic" }, + { id: "claude-3-7-sonnet-20250219", provider: "anthropic" }, // dated → claude-3-7-sonnet + { id: "claude-opus-4", provider: "anthropic" }, + { id: "claude-opus-4-20250514", provider: "anthropic" }, // dated → claude-opus-4 + { id: "claude-sonnet-4", provider: "anthropic" }, + { id: "claude-haiku-4", provider: "anthropic" }, + + // ── Gemini ──────────────────────────────────────────────────────────────── + { id: "gemini-1.5-pro", provider: "gemini" }, + { id: "gemini-1.5-pro-2024-05-14", provider: "gemini" }, // dated → gemini-1.5-pro + { id: "gemini-1.5-flash", provider: "gemini" }, + { id: "gemini-2.5-flash", provider: "gemini" }, + { id: "gemini-2.5-pro", provider: "gemini" }, + { id: "gemini-2.0-flash-exp", provider: "gemini" }, // exclude: experimental + { id: "gemini-2.0-flash-thinking-exp", provider: "gemini" }, // exclude: experimental +]; + +/** + * Provider-mode tokens that aimock uses internally to route to a real upstream + * provider but that are NOT model ids any provider's /models endpoint exposes. + * These MUST be on NON_MODEL_TOKENS — they must never be mistaken for real ids. + */ +const NON_MODEL_TOKENS_UNDER_TEST: string[] = ["gemini-interactions"]; + +describe("§6.1c builder/fixture cross-check (no live keys)", () => { + it("every referenced builder/fixture model id resolves to a classified family", () => { + const failures: string[] = []; + + for (const { id, provider } of BUILDER_FIXTURE_MODEL_IDS) { + const family = normalizeModelFamily(id, provider); + const inInclude = includeFamilies[provider].has(family); + const inExclude = excludeFamilies[provider].has(family); + + if (!inInclude && !inExclude) { + failures.push( + `${id} (${provider}): normalized to "${family}" which is in NEITHER includeFamilies NOR excludeFamilies`, + ); + } + } + + expect( + failures, + `Stray builder/fixture model ids not in registry:\n${failures.join("\n")}`, + ).toEqual([]); + }); + + it("every non-model provider-mode token is on NON_MODEL_TOKENS", () => { + const failures: string[] = []; + + for (const token of NON_MODEL_TOKENS_UNDER_TEST) { + if (!NON_MODEL_TOKENS.has(token)) { + failures.push( + `"${token}" is not on NON_MODEL_TOKENS — a greedy scrape would false-positive`, + ); + } + } + + expect( + failures, + `Provider-mode tokens missing from NON_MODEL_TOKENS:\n${failures.join("\n")}`, + ).toEqual([]); + }); +}); diff --git a/src/__tests__/drift/models-scrape.drift.ts b/src/__tests__/drift/models-scrape.drift.ts deleted file mode 100644 index d6b00896..00000000 --- a/src/__tests__/drift/models-scrape.drift.ts +++ /dev/null @@ -1,48 +0,0 @@ -/** - * Regression guard for the model-id scraper used by the drift checks. - * - * The Gemini availability check scrapes `gemini-*` tokens from source files — - * including README.md — with a greedy regex, then checks each against Google's - * live model list. aimock also has "provider modes" such as `gemini-interactions` - * that route to the Gemini upstream API but are NOT Gemini model ids. When the - * README documented such a mode as a bare `gemini-interactions` token, the - * greedy scraper grabbed it, the availability check failed (it is not a real - * Google model), and the drift collector crashed as an "unparseable" failure — - * firing a daily false-positive drift alert with no real drift. - * - * Two layers of defense are guarded here against the REAL scrape + filter - * surface (real README, real regex, real filter): - * 1. The stable filter drops aimock provider-mode names even if scraped. - * 2. The README no longer spells the mode as a bare model-id token, so the - * scraper never picks it up in the first place. - */ - -import { describe, it, expect } from "vitest"; -import { - scrapeModels, - filterStableGeminiModels, - sourceFiles, - GEMINI_MODEL_PATTERN, -} from "./models.drift.js"; - -describe("Gemini model scrape does not flag aimock provider-mode names", () => { - it("stable filter drops the gemini-interactions provider-mode token", () => { - // Layer 1 (the real fix): even if a `gemini-interactions` token is scraped - // from anywhere, the stable filter must exclude it so it is never checked - // for drift. This is independent of any doc wording. - const stable = filterStableGeminiModels([ - "gemini-2.5-flash", - "gemini-interactions", - "gemini-1.5-pro", - ]); - expect(stable).toEqual(["gemini-2.5-flash", "gemini-1.5-pro"]); - }); - - it("real source-file scrape produces no provider-mode false positives", () => { - // Layer 2: run the exact scrape + filter the drift check uses over the - // real source files. The result must contain no aimock provider-mode names. - const referenced = scrapeModels(GEMINI_MODEL_PATTERN, sourceFiles); - const stable = filterStableGeminiModels(referenced); - expect(stable).not.toContain("gemini-interactions"); - }); -}); From ca873e0df532c42d177f708b4bb018da53fbb269 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 22:24:45 -0700 Subject: [PATCH 15/24] feat(drift): wire numeric HTTP status onto InfraError (C5.1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce `InfraError extends Error` with a first-class readonly `status: number` field. Update `assertOk` to throw `InfraError(msg, status)` and `withInfraErrorTag` to propagate the numeric status on re-wrap, so downstream classifiers (Slack alerts C5.2, preflight C5.3a) can key off `status === 401 || 403 → stale-key` vs `429/5xx → infra-transient` without any prose-message parsing. Network errors that never reach assertOk have no HTTP status; they remain plain Errors with status 0 on the InfraError wrapper. --- .../drift/providers-infra-error.test.ts | 77 +++++++++++++++++++ src/__tests__/drift/providers.ts | 29 ++++++- 2 files changed, 104 insertions(+), 2 deletions(-) create mode 100644 src/__tests__/drift/providers-infra-error.test.ts diff --git a/src/__tests__/drift/providers-infra-error.test.ts b/src/__tests__/drift/providers-infra-error.test.ts new file mode 100644 index 00000000..34019683 --- /dev/null +++ b/src/__tests__/drift/providers-infra-error.test.ts @@ -0,0 +1,77 @@ +/** + * Unit test for structured numeric `status` on InfraError (C5.1). + * + * Verifies that errors thrown through the `assertOk` → `withInfraErrorTag` + * pipeline carry a first-class numeric `.status` field, so downstream + * classification (C5.2 Slack, C5.3a preflight) can key off + * `status === 401 || status === 403` → stale-key vs `429`/`5xx` → infra-transient + * WITHOUT parsing prose message strings. + * + * RED captured (pre-change): `e.status` was `undefined` — no numeric status. + * GREEN (post-change): `e.status` is the exact HTTP status code. + */ +import { describe, it, expect } from "vitest"; +import { listOpenAIModels } from "./providers.js"; + +// Helper: build a minimal Response-like object +function fakeResponse(status: number, body = "Unauthorized"): Response { + return new Response(body, { status }); +} + +describe("InfraError structured status (C5.1)", () => { + it("GREEN: caught error exposes status === 401 (stale-key class)", async () => { + const origFetch = globalThis.fetch; + globalThis.fetch = async () => fakeResponse(401, "Unauthorized"); + try { + await listOpenAIModels("fake-key"); + throw new Error("should have thrown"); + } catch (err: unknown) { + expect(err).toBeInstanceOf(Error); + const e = err as Error & { status?: number }; + // Must be numeric 401, not undefined + expect(typeof e.status).toBe("number"); + expect(e.status).toBe(401); + // Human-readable INFRA_ERROR prose must still be present in message + expect((e as Error).message).toMatch(/INFRA_ERROR/); + } finally { + globalThis.fetch = origFetch; + } + }); + + it("GREEN: caught error exposes status === 403 (stale-key class)", async () => { + const origFetch = globalThis.fetch; + globalThis.fetch = async () => fakeResponse(403, "Forbidden"); + try { + await listOpenAIModels("fake-key"); + throw new Error("should have thrown"); + } catch (err: unknown) { + expect(err).toBeInstanceOf(Error); + const e = err as Error & { status?: number }; + expect(typeof e.status).toBe("number"); + expect(e.status).toBe(403); + expect((e as Error).message).toMatch(/INFRA_ERROR/); + } finally { + globalThis.fetch = origFetch; + } + }); + + it("GREEN: caught error exposes status === 503 (infra-transient class)", async () => { + const origFetch = globalThis.fetch; + // 503 is in RETRYABLE_STATUSES; fetchWithRetry retries attempts 0 and 1, + // then returns the response on attempt 2 (the last) without further retry. + // assertOk then fires on the 503 response. + globalThis.fetch = async () => fakeResponse(503, "Service Unavailable"); + try { + await listOpenAIModels("fake-key"); + throw new Error("should have thrown"); + } catch (err: unknown) { + expect(err).toBeInstanceOf(Error); + const e = err as Error & { status?: number }; + expect(typeof e.status).toBe("number"); + expect(e.status).toBe(503); + expect((e as Error).message).toMatch(/INFRA_ERROR/); + } finally { + globalThis.fetch = origFetch; + } + }); +}); diff --git a/src/__tests__/drift/providers.ts b/src/__tests__/drift/providers.ts index 03c80967..c2b2c309 100644 --- a/src/__tests__/drift/providers.ts +++ b/src/__tests__/drift/providers.ts @@ -27,6 +27,27 @@ interface StreamResult { rawEvents: { type: string; data: unknown }[]; } +/** + * Structured error carrying the HTTP status code as a first-class numeric field. + * + * Downstream classifiers (Slack alerts, preflight checks) MUST key off + * `error.status` — never parse the prose message string. + * + * 401 | 403 → stale-key (replace the API key) + * 429 → rate-limited (back off) + * 5xx → infra-transient (retry / alert ops) + * network → no status field (instanceof check first) + */ +export class InfraError extends Error { + readonly status: number; + + constructor(message: string, status: number) { + super(message); + this.name = "InfraError"; + this.status = status; + } +} + // --------------------------------------------------------------------------- // Retry helper // --------------------------------------------------------------------------- @@ -71,7 +92,10 @@ function redactUrl(url: string): string { function assertOk(raw: string, status: number, context: string, url?: string): void { if (status >= 400) { const urlSuffix = url ? ` (${redactUrl(url)})` : ""; - throw new Error(`${context}: API returned ${status}${urlSuffix}: ${raw.slice(0, 300)}`); + throw new InfraError( + `${context}: API returned ${status}${urlSuffix}: ${raw.slice(0, 300)}`, + status, + ); } } @@ -157,7 +181,8 @@ function toSSEEventShapes(events: { type: string; data: unknown }[]): SSEEventSh function withInfraErrorTag(provider: string, fn: () => Promise): Promise { return fn().catch((err: unknown) => { const msg = err instanceof Error ? err.message : String(err); - throw new Error(`INFRA_ERROR: ${provider} — ${msg}`); + const status = err instanceof InfraError ? err.status : 0; + throw new InfraError(`INFRA_ERROR: ${provider} — ${msg}`, status); }); } From 6884ae1797290eefaca37184756f1c4512cfa648 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 22:32:42 -0700 Subject: [PATCH 16/24] =?UTF-8?q?feat(drift):=20C5.2=20=E2=80=94=20headlin?= =?UTF-8?q?e=20class=20+=20per-item=20detail=20in=20summarizeDriftReport?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Upgrade summarizeDriftReport to: - Compute a headline class (real-drift / stale-key / infra-transient / quarantine / test-infra-false-positive) from InfraError status, report entries severity, and quarantine[] presence. Priority: stale-key > infra-transient > real-drift > quarantine > test-infra-false-positive. Critical wins over quarantine (mirrors computeExitCode contract). - Emit per-item detail: provider / id-or-path / builderFile / issue text. - Render a distinct quarantine section for report.quarantine[] entries, with testName + rawLocation (file:line) + truncated message. - Accept optional SummarizeOptions { infraErrorStatus?, exitCode? } for classification context; zero-opt callers and empty reports preserve the historical empty-string return. - Export HeadlineClass and SummarizeOptions types for downstream use. The drift_summary GITHUB_OUTPUT key is unchanged — callers write the full return value verbatim; the workflow's DRIFT_SUMMARY env-var receives the augmented mrkdwn string. Verified: test-drift.yml reads only steps.summary.outputs.drift_summary (line 42) via writeGithubOutput, which is invoked unchanged in main() with the summary as the sole value. Golden-file snapshot tests cover all five classes (real-drift, stale-key, infra-transient, quarantine, test-infra-false-positive), per-item id preference over path, quarantine distinct-line format, and mixed entries+quarantine. 10 new tests; all 4386 tests pass, tsc clean, lint clean. --- scripts/drift-slack-summary.ts | 172 +++++++++++++++++++++++++--- src/__tests__/drift-scripts.test.ts | 159 ++++++++++++++++++++++++- 2 files changed, 306 insertions(+), 25 deletions(-) diff --git a/scripts/drift-slack-summary.ts b/scripts/drift-slack-summary.ts index 6fb5412a..6b705477 100644 --- a/scripts/drift-slack-summary.ts +++ b/scripts/drift-slack-summary.ts @@ -29,6 +29,54 @@ import { fileURLToPath } from "node:url"; import { readDriftReport } from "./fix-drift.js"; import type { DriftEntry, DriftReport, DriftSeverity } from "./drift-types.js"; +// --------------------------------------------------------------------------- +// Headline classification +// --------------------------------------------------------------------------- + +/** + * Closed headline-class enum for a drift run. + * + * Derivation priority (highest wins): + * stale-key — InfraError status 401 or 403 + * infra-transient — InfraError status 429 or ≥500 + * quarantine — report.quarantine[] is non-empty (exit 5) + * real-drift — at least one entry has a critical diff (exit 2) + * test-infra-false-positive — everything else (exit 0 / advisory only) + */ +export type HeadlineClass = + | "real-drift" + | "test-infra-false-positive" + | "infra-transient" + | "stale-key" + | "quarantine"; + +/** Optional context the caller may supply to sharpen classification. */ +export interface SummarizeOptions { + /** HTTP status from an InfraError that aborted the run, if any. */ + infraErrorStatus?: number; + /** Process exit code produced by the collector/retry wrapper, if known. */ + exitCode?: number; +} + +function computeHeadlineClass(report: DriftReport, opts: SummarizeOptions): HeadlineClass { + const { infraErrorStatus } = opts; + + if (infraErrorStatus === 401 || infraErrorStatus === 403) return "stale-key"; + if (infraErrorStatus === 429 || (infraErrorStatus !== undefined && infraErrorStatus >= 500)) + return "infra-transient"; + + // Critical drift wins over quarantine — a confirmed critical finding is the + // actionable signal even when some failures were also quarantined (mirrors the + // computeExitCode contract: crit→2 wins over quarantine→5). + const hasCritical = report.entries.some((e) => e.diffs.some((d) => d.severity === "critical")); + if (hasCritical) return "real-drift"; + + const hasQuarantine = (report.quarantine?.length ?? 0) > 0; + if (hasQuarantine) return "quarantine"; + + return "test-infra-false-positive"; +} + // Keep the message scannable: cap how many example paths we list per provider // and how many total providers we enumerate before collapsing to a count. const MAX_PATHS_PER_PROVIDER = 3; @@ -39,12 +87,19 @@ const SEVERITY_ORDER: DriftSeverity[] = ["critical", "warning", "info"]; interface ProviderSummary { provider: string; counts: Record; + /** Ordered list of changed path strings (de-duplicated). */ paths: string[]; + /** Ordered list of per-diff ids (de-duplicated), when present. */ + ids: string[]; + /** The builderFile from the first matching entry, for the file reference. */ + builderFile: string; + /** The issue text from the first diff, for one-line context. */ + firstIssue: string; } /** * Group drift entries by provider, tallying severities and collecting a small, - * de-duplicated set of representative changed paths. + * de-duplicated set of representative changed paths and ids. */ function summarizeByProvider(entries: DriftEntry[]): ProviderSummary[] { const byProvider = new Map(); @@ -56,14 +111,23 @@ function summarizeByProvider(entries: DriftEntry[]): ProviderSummary[] { provider: entry.provider, counts: { critical: 0, warning: 0, info: 0 }, paths: [], + ids: [], + builderFile: entry.builderFile, + firstIssue: "", }; byProvider.set(entry.provider, summary); } for (const diff of entry.diffs) { summary.counts[diff.severity]++; + if (diff.id && !summary.ids.includes(diff.id)) { + summary.ids.push(diff.id); + } if (diff.path && !summary.paths.includes(diff.path)) { summary.paths.push(diff.path); } + if (!summary.firstIssue && diff.issue) { + summary.firstIssue = diff.issue; + } } } @@ -75,6 +139,18 @@ function summarizeByProvider(entries: DriftEntry[]): ProviderSummary[] { }); } +/** + * For a provider summary, return the ordered list of identifiers to use as + * per-item references in the Slack bullet. + * + * When any diff carries a stable `id` (e.g. a model id), prefer those over + * raw paths — they're more human-readable in a Slack alert. Fall back to paths + * when no ids are present. + */ +function buildIdOrPaths(s: ProviderSummary): string[] { + return s.ids.length > 0 ? s.ids : s.paths; +} + /** Format a severity tally like "2 critical, 1 warning" (omitting zeroes). */ function formatCounts(counts: Record): string { const parts: string[] = []; @@ -87,32 +163,90 @@ function formatCounts(counts: Record): string { /** * Build the Slack mrkdwn summary lines for the drifted providers. * - * Returns a single string with real `\n` line breaks. Each provider gets a - * bullet line: `• *Provider* — 2 critical: \`path.a\`, \`path.b\``. - * Returns an empty string when there are no entries (caller decides fallback). + * Returns a single string with real `\n` line breaks. The first line is a + * headline classification (`*Classification:* `), followed by per-item + * bullets in one of two forms depending on the class: + * + * Real-drift/advisory: + * `• *Provider* — 2 critical: \`path.a\`, \`path.b\` (src/helpers.ts)` + * + * Quarantine: + * `• [quarantine] Provider — testName (file:line): truncated message` + * + * Returns an empty string only when no entries, no quarantine, no InfraError + * context, and no exitCode hint are present (i.e. caller has nothing to say). + * + * The `drift_summary` GITHUB_OUTPUT key is the full return value of this + * function — callers write it verbatim to `GITHUB_OUTPUT` via writeGithubOutput. + * Existing consumers that read `steps.summary.outputs.drift_summary` receive + * the augmented text; they pattern-match on its content (not on internal + * structure), so the addition of the classification line is backward-compatible. */ -export function summarizeDriftReport(report: DriftReport): string { +export function summarizeDriftReport(report: DriftReport, opts: SummarizeOptions = {}): string { + const headlineClass = computeHeadlineClass(report, opts); + const summaries = summarizeByProvider(report.entries); - if (summaries.length === 0) return ""; + const quarantine = report.quarantine ?? []; + + // Nothing to say at all — preserve historical empty-string behaviour for a + // clean run with no context supplied. + if ( + summaries.length === 0 && + quarantine.length === 0 && + headlineClass === "test-infra-false-positive" && + opts.infraErrorStatus === undefined && + opts.exitCode === undefined + ) { + return ""; + } - const shown = summaries.slice(0, MAX_PROVIDERS_LISTED); const lines: string[] = []; - for (const s of shown) { - const examplePaths = s.paths.slice(0, MAX_PATHS_PER_PROVIDER).map((p) => `\`${p}\``); - const extraPaths = s.paths.length - examplePaths.length; - let pathStr = examplePaths.join(", "); - if (extraPaths > 0) pathStr += `, +${extraPaths} more`; + // ── Headline classification ──────────────────────────────────────────────── + lines.push(`*Classification:* ${headlineClass}`); - const counts = formatCounts(s.counts); - lines.push( - pathStr ? `• *${s.provider}* — ${counts}: ${pathStr}` : `• *${s.provider}* — ${counts}`, - ); + // ── Per-entry drift bullets (real-drift / advisory) ─────────────────────── + if (summaries.length > 0) { + const shown = summaries.slice(0, MAX_PROVIDERS_LISTED); + + for (const s of shown) { + // Per-item: prefer id over path when present on the first diff that has one. + const idOrPaths = buildIdOrPaths(s); + const exampleRefs = idOrPaths.slice(0, MAX_PATHS_PER_PROVIDER).map((r) => `\`${r}\``); + const extraRefs = idOrPaths.length - exampleRefs.length; + let refStr = exampleRefs.join(", "); + if (extraRefs > 0) refStr += `, +${extraRefs} more`; + + // Per-item: file reference from the entry's builderFile. + const fileRef = s.builderFile ? ` (${s.builderFile})` : ""; + + // Per-item: first one-line issue text. + const issueStr = s.firstIssue ? ` — _${s.firstIssue}_` : ""; + + const counts = formatCounts(s.counts); + lines.push( + refStr + ? `• *${s.provider}* — ${counts}: ${refStr}${issueStr}${fileRef}` + : `• *${s.provider}* — ${counts}${issueStr}${fileRef}`, + ); + } + + const hiddenProviders = summaries.length - shown.length; + if (hiddenProviders > 0) { + lines.push(`• …and ${hiddenProviders} more provider${hiddenProviders === 1 ? "" : "s"}`); + } } - const hiddenProviders = summaries.length - shown.length; - if (hiddenProviders > 0) { - lines.push(`• …and ${hiddenProviders} more provider${hiddenProviders === 1 ? "" : "s"}`); + // ── Quarantine entries ───────────────────────────────────────────────────── + if (quarantine.length > 0) { + lines.push( + `*Quarantined* (${quarantine.length} failure${quarantine.length === 1 ? "" : "s"} need human review):`, + ); + for (const q of quarantine) { + const loc = q.rawLocation ? ` (${q.rawLocation})` : ""; + const msg = q.message.slice(0, 120); + lines.push(`• [quarantine] *${q.provider}* — ${q.testName}${loc}: ${msg}`); + } } return lines.join("\n"); diff --git a/src/__tests__/drift-scripts.test.ts b/src/__tests__/drift-scripts.test.ts index 268fb68d..6ce34527 100644 --- a/src/__tests__/drift-scripts.test.ts +++ b/src/__tests__/drift-scripts.test.ts @@ -19,7 +19,7 @@ import { import { summarizeDriftReport } from "../../scripts/drift-slack-summary.js"; -import type { DriftEntry, DriftReport } from "../../scripts/drift-types.js"; +import type { DriftEntry, DriftReport, QuarantineEntry } from "../../scripts/drift-types.js"; // --------------------------------------------------------------------------- // Helpers @@ -356,7 +356,9 @@ describe("summarizeDriftReport", () => { expect(summary).toContain("*OpenAI Chat*"); expect(summary).toContain("1 critical"); expect(summary).toContain("`choices[0].message.refusal`"); - expect(summary.startsWith("•")).toBe(true); + // The first line is now the classification header; bullet lines follow. + const bulletLines = summary.split("\n").filter((l) => l.startsWith("•")); + expect(bulletLines.length).toBeGreaterThan(0); }); it("merges multiple entries for the same provider into one line with combined counts", () => { @@ -404,11 +406,12 @@ describe("summarizeDriftReport", () => { makeEntry(), // OpenAI Chat with a critical ], }); - const lines = summary.split("\n"); - expect(lines).toHaveLength(2); + // First line is the classification header; provider bullets follow. + const bulletLines = summary.split("\n").filter((l) => l.startsWith("•")); + expect(bulletLines).toHaveLength(2); // Provider with a critical diff sorts before the warning-only provider - expect(lines[0]).toContain("*OpenAI Chat*"); - expect(lines[1]).toContain("*Anthropic*"); + expect(bulletLines[0]).toContain("*OpenAI Chat*"); + expect(bulletLines[1]).toContain("*Anthropic*"); }); it("caps example paths per provider and reports the remainder", () => { @@ -439,3 +442,147 @@ describe("summarizeDriftReport", () => { expect(summary).not.toContain("\\n"); }); }); + +// --------------------------------------------------------------------------- +// summarizeDriftReport — headline class + per-item detail (C5.2) +// --------------------------------------------------------------------------- + +function makeQuarantineEntry(overrides?: Partial): QuarantineEntry { + return { + provider: "OpenAI Chat", + testName: "OpenAI Chat > non-streaming text > response shape", + rawLocation: "src/__tests__/drift/openai-chat.drift.ts:42", + message: "Cannot read properties of undefined (reading 'choices')", + ...overrides, + }; +} + +describe("summarizeDriftReport — headline class", () => { + it("class: real-drift — report has entries with critical diffs", () => { + const report: DriftReport = { + timestamp: "t", + entries: [ + makeEntry({ + diffs: [ + { + severity: "critical", + issue: "field missing from mock", + path: "choices[0].message.refusal", + expected: "null", + real: "null", + mock: "", + }, + ], + }), + ], + }; + const summary = summarizeDriftReport(report); + expect(summary).toContain("real-drift"); + // per-item: provider name present + expect(summary).toContain("OpenAI Chat"); + // per-item: offending path or id + expect(summary).toContain("choices[0].message.refusal"); + // per-item: one-line issue + expect(summary).toContain("field missing from mock"); + // per-item: file reference (builderFile) + expect(summary).toContain("src/helpers.ts"); + }); + + it("class: quarantine — report has quarantine[] entries", () => { + const report: DriftReport = { + timestamp: "t", + entries: [], + quarantine: [makeQuarantineEntry()], + }; + const summary = summarizeDriftReport(report); + expect(summary).toContain("quarantine"); + // quarantine: provider + expect(summary).toContain("OpenAI Chat"); + // quarantine: rawLocation (file:line) + expect(summary).toContain("src/__tests__/drift/openai-chat.drift.ts:42"); + // quarantine: one-line message + expect(summary).toContain("Cannot read properties of undefined"); + }); + + it("class: stale-key — InfraError status 401", () => { + const report: DriftReport = { timestamp: "t", entries: [] }; + const summary = summarizeDriftReport(report, { infraErrorStatus: 401 }); + expect(summary).toContain("stale-key"); + }); + + it("class: stale-key — InfraError status 403", () => { + const report: DriftReport = { timestamp: "t", entries: [] }; + const summary = summarizeDriftReport(report, { infraErrorStatus: 403 }); + expect(summary).toContain("stale-key"); + }); + + it("class: infra-transient — InfraError status 429", () => { + const report: DriftReport = { timestamp: "t", entries: [] }; + const summary = summarizeDriftReport(report, { infraErrorStatus: 429 }); + expect(summary).toContain("infra-transient"); + }); + + it("class: infra-transient — InfraError status 503", () => { + const report: DriftReport = { timestamp: "t", entries: [] }; + const summary = summarizeDriftReport(report, { infraErrorStatus: 503 }); + expect(summary).toContain("infra-transient"); + }); + + it("class: test-infra-false-positive — exit 0, empty entries, no infraError", () => { + const report: DriftReport = { timestamp: "t", entries: [] }; + const summary = summarizeDriftReport(report, { exitCode: 0 }); + expect(summary).toContain("test-infra-false-positive"); + }); + + it("quarantine entries appear on their own distinct line with testName", () => { + const report: DriftReport = { + timestamp: "t", + entries: [], + quarantine: [ + makeQuarantineEntry({ provider: "Anthropic", testName: "Anthropic > streaming > shape" }), + ], + }; + const summary = summarizeDriftReport(report); + const lines = summary.split("\n"); + const quarantineLine = lines.find((l) => l.includes("Anthropic")); + expect(quarantineLine).toBeDefined(); + expect(quarantineLine).toContain("Anthropic > streaming > shape"); + }); + + it("mixed: real-drift entries + quarantine both appear in summary", () => { + const report: DriftReport = { + timestamp: "t", + entries: [makeEntry()], + quarantine: [makeQuarantineEntry({ provider: "Gemini" })], + }; + const summary = summarizeDriftReport(report); + // Class is real-drift when entries have critical diffs (quarantine is secondary) + expect(summary).toContain("real-drift"); + // Quarantine section still present + expect(summary).toContain("Gemini"); + expect(summary).toContain("quarantine"); + }); + + it("per-item id is used over path when present", () => { + const report: DriftReport = { + timestamp: "t", + entries: [ + makeEntry({ + diffs: [ + { + severity: "critical", + issue: "model removed from mock", + path: "knownModels", + id: "gpt-4o-mini", + expected: "present", + real: "present", + mock: "", + }, + ], + }), + ], + }; + const summary = summarizeDriftReport(report); + expect(summary).toContain("gpt-4o-mini"); + }); +}); From dcbf7476fdc535d81029c08366889055678e043d Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 22:38:21 -0700 Subject: [PATCH 17/24] =?UTF-8?q?feat(drift):=20C5.3a=20=E2=80=94=20per-pr?= =?UTF-8?q?ovider=20key-freshness=20preflight=20(stale-key)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a preflight step at the start of the `drift` job: a cheap GET /models per provider (OpenAI/Anthropic/Gemini) before the 15-minute suite. A dead or revoked key returns 401/403, which the step classifies as `stale-key` and fails LOUD with a "rotate _API_KEY" message — never a silent green skip — so a dead key surfaces up front instead of as a late generic streaming failure. Classification keys off the numeric InfraError.status (401/403), not the prose message. Transient statuses (429/5xx/network) are not stale keys: the preflight warns and continues, since the retry-wrapped collector handles those. The preflight program is written to the checkout root before running so its relative provider import resolves against the repo (ESM relative specifiers key off the module location, not cwd); cleaned up on exit. --- .github/workflows/test-drift.yml | 83 ++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/.github/workflows/test-drift.yml b/.github/workflows/test-drift.yml index 4aed033e..2f7b9a42 100644 --- a/.github/workflows/test-drift.yml +++ b/.github/workflows/test-drift.yml @@ -54,6 +54,89 @@ jobs: cache: pnpm - run: pnpm install --frozen-lockfile + # Per-provider key-freshness PREFLIGHT. Before spending 15 minutes on the + # full suite, do a cheap GET /models per provider. A dead/expired/revoked + # key returns 401/403 — classify that as `stale-key` and fail the job LOUD + # with a "rotate _API_KEY" message, so a dead key surfaces up + # front instead of as a late, generic streaming failure buried in the + # suite (or worse, a misleading green skip). Classification keys off the + # numeric InfraError.status (401/403), NEVER the prose message. Transient + # statuses (429/5xx/network) are NOT stale keys: the preflight warns and + # continues, since the retry-wrapped collector below handles those. + # (C5.3b, a separate task, adds the exit_code job-output + notify + # plumbing — this step is only the preflight.) + - name: Preflight — provider key freshness + env: + OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + GOOGLE_API_KEY: ${{ secrets.GOOGLE_API_KEY }} + run: | + # Written at the checkout root (cwd) so the relative provider import + # below resolves against the repo. ESM relative specifiers key off the + # module's own location, not the process cwd — a file under + # $RUNNER_TEMP would look for src/ beside itself and fail. Cleaned up + # on exit either way. + PREFLIGHT_FILE="$GITHUB_WORKSPACE/.drift-preflight.mts" + trap 'rm -f "$PREFLIGHT_FILE"' EXIT + cat > "$PREFLIGHT_FILE" <<'PREFLIGHT' + import { + InfraError, + listOpenAIModels, + listAnthropicModels, + listGeminiModels, + } from "./src/__tests__/drift/providers.js"; + + const PROVIDERS: { + name: string; + keyEnv: string; + list: (key: string) => Promise; + }[] = [ + { name: "OpenAI", keyEnv: "OPENAI_API_KEY", list: listOpenAIModels }, + { name: "Anthropic", keyEnv: "ANTHROPIC_API_KEY", list: listAnthropicModels }, + { name: "Gemini", keyEnv: "GOOGLE_API_KEY", list: listGeminiModels }, + ]; + + const stale: string[] = []; + + for (const p of PROVIDERS) { + const key = process.env[p.keyEnv]; + if (!key) { + console.error(`::error::${p.keyEnv} is not set — cannot run drift for ${p.name}`); + stale.push(p.keyEnv); + continue; + } + try { + await p.list(key); + console.log(`preflight: ${p.name} key OK`); + } catch (err) { + const status = err instanceof InfraError ? err.status : 0; + if (status === 401 || status === 403) { + // stale-key: the key is dead/expired/revoked. LOUD, classified. + console.error( + `::error title=stale-key::${p.name} preflight got HTTP ${status} — rotate ${p.keyEnv}`, + ); + stale.push(p.keyEnv); + } else { + // Transient (429/5xx/network) at preflight time is NOT a stale + // key — don't block; the retry-wrapped collector handles it. + const msg = err instanceof Error ? err.message : String(err); + console.warn( + `preflight: ${p.name} non-auth error (status ${status}), continuing: ${msg}`, + ); + } + } + } + + if (stale.length > 0) { + console.error( + `::error::Preflight failed (stale-key): rotate ${stale.join(", ")} before drift can run`, + ); + process.exit(1); + } + console.log("preflight: all provider keys fresh"); + PREFLIGHT + npx tsx "$PREFLIGHT_FILE" + # Run the collector behind a "retry before alert" wrapper. A single # critical run can be a transient real-API hiccup (a streaming call # failing mid-flight), NOT a format change — so the wrapper re-runs the From 6047fa6487b55257b5a35bf28a3d48731e59704c Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 22:41:49 -0700 Subject: [PATCH 18/24] =?UTF-8?q?feat(drift):=20C5.3b=20=E2=80=94=20promot?= =?UTF-8?q?e=20collector=20exit=5Fcode=20to=20job=20output=20+=20notify=20?= =?UTF-8?q?quarantine=20class?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Promote the drift collector's exit_code from a step output to a drift-job JOB-level output (alongside summary/runs) so the separate notify job can read needs.drift.outputs.exit_code. In the notify job, classify exit_code==5 (collector quarantined unparseable output) as a distinct quarantine alert BEFORE the generic DRIFT_RESULT==failure -> HTTP_DRIFT=true fallback. Without this, an exit-5 quarantine (drift job concludes failure) was misreported as real HTTP API drift (providers changed formats), sending readers chasing a phantom format change instead of doing the manual triage a quarantine needs. Exit 2 (real critical drift) and normal failures still route to the HTTP-drift/real-drift path unchanged. M5 backstop per ironclad spec 6.4. --- .github/workflows/test-drift.yml | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test-drift.yml b/.github/workflows/test-drift.yml index 2f7b9a42..c99dd2b3 100644 --- a/.github/workflows/test-drift.yml +++ b/.github/workflows/test-drift.yml @@ -44,6 +44,11 @@ jobs: # persisted across every retry). Lets the alert say "confirmed across N # runs". Empty on the common green/transient path. runs: ${{ steps.drift.outputs.drift_runs }} + # Final collector exit code (0 clean/transient, 2 critical drift, 5 + # quarantine, other = crash). Promoted to a job output so the `notify` + # job can classify an exit-5 quarantine distinctly instead of folding it + # into the generic "job failed → HTTP drift" fallback (M5 backstop). + exit_code: ${{ steps.drift.outputs.exit_code }} steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: { persist-credentials: false } @@ -215,6 +220,7 @@ jobs: AGUI_RESULT: ${{ needs.agui-schema-drift.result }} DRIFT_SUMMARY: ${{ needs.drift.outputs.summary }} DRIFT_RUNS: ${{ needs.drift.outputs.runs }} + DRIFT_EXIT_CODE: ${{ needs.drift.outputs.exit_code }} REPO: ${{ github.repository }} RUN_ID: ${{ github.run_id }} run: | @@ -223,9 +229,17 @@ jobs: HTTP_DRIFT=false AGUI_DRIFT=false INFRA_ERROR=false + QUARANTINE=false - # Determine what happened in each job - if [ "$DRIFT_RESULT" = "failure" ]; then + # Determine what happened in each job. An exit_code of 5 means the + # collector QUARANTINED unparseable output — the drift job concludes + # `failure`, but this is NOT real HTTP API drift: it needs human + # triage, not a "providers changed formats" alert. Classify it as + # quarantine FIRST, before the generic failure→HTTP_DRIFT fallback, + # so an exit-5 quarantine is never misreported as real drift. + if [ "$DRIFT_EXIT_CODE" = "5" ]; then + QUARANTINE=true + elif [ "$DRIFT_RESULT" = "failure" ]; then HTTP_DRIFT=true elif [ "$DRIFT_RESULT" != "success" ] && [ "$DRIFT_RESULT" != "skipped" ]; then INFRA_ERROR=true @@ -261,8 +275,15 @@ jobs: CONFIRMED="${NL}_(confirmed across ${DRIFT_RUNS} runs)_" fi + # Quarantine (collector exit 5): unparseable output was quarantined + # and needs manual triage. Classified distinctly — NOT real HTTP API + # drift — so nobody chases a phantom provider format change. The + # DRIFT_SUMMARY already carries the quarantine detail block (C5.2). + if [ "$QUARANTINE" = "true" ]; then + EMOJI="🔬" + MSG="*Drift collector quarantined unparseable output* in aimock — manual triage required (not confirmed API drift).${DETAIL}${NL}${RUN_URL}" # Both types of drift - if [ "$HTTP_DRIFT" = "true" ] && [ "$AGUI_DRIFT" = "true" ]; then + elif [ "$HTTP_DRIFT" = "true" ] && [ "$AGUI_DRIFT" = "true" ]; then EMOJI="🚨" MSG="*Drift detected* in aimock — HTTP API drift + AG-UI schema drift.${DETAIL}${CONFIRMED}${NL}${RUN_URL}" # HTTP API drift only From 6c84dc90c0f65e5b53ffff6b7b376f6d9518fff9 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 22:50:03 -0700 Subject: [PATCH 19/24] =?UTF-8?q?feat(drift):=20D6.2=20=E2=80=94=20populat?= =?UTF-8?q?e=20per-item=20id=20on=20ParsedDiff=20for=20delta=20keying?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Set `id` on every ParsedDiff produced by the collector so downstream delta logic (D6.1) can key findings by provider+id: - parseDriftBlock: `id = path` (stable per-item key from the offending field path; different paths → different delta keys) - canary CLASS 3 (unknown-models): `id = ` (the value already in `diff.real`), so N distinct unknown model ids → N distinct delta keys rather than collapsing under a shared `path:"knownModels"` bucket - canary CLASS 2 / noGA unknownIds spread: same `id = ` treatment Additive only — no existing field removed; exit-code/quarantine logic (computeExitCode, collectDriftEntries quarantine path) is untouched. RED: 3 unknown model ids → all diffs had `id === undefined` → 1 collapsed key GREEN: 3 unknown model ids → 3 distinct `id` values → 3 distinct delta keys --- scripts/drift-report-collector.ts | 14 ++++- src/__tests__/drift-collector.test.ts | 82 +++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/scripts/drift-report-collector.ts b/scripts/drift-report-collector.ts index 26417a3f..1108a8c4 100644 --- a/scripts/drift-report-collector.ts +++ b/scripts/drift-report-collector.ts @@ -207,13 +207,17 @@ export function parseDriftBlock(text: string): { context: string; diffs: ParsedD ); continue; } + const path = match[3].trim(); diffs.push({ severity: severity as DriftSeverity, issue: match[2].trim(), - path: match[3].trim(), + path, expected: match[4].trim(), real: match[5].trim(), mock: match[6].trim(), + // Stable per-item key derived from the offending path so different paths + // yield different delta keys (provider+id) in the D6.1 delta layer. + id: path, }); } @@ -726,6 +730,10 @@ export function collectDriftEntries(results: VitestJsonResult): CollectResult { expected: "(not in knownModels set)", real: id, mock: NO_MOCK_LEG, + // D6.2: set `id` to the model id so the delta layer (D6.1) + // can key by provider+id, yielding one distinct key per model + // rather than collapsing all entries under path:"knownModels". + id, })), ] : // CLASS 3: only genuine model ids become `real` diffs. The @@ -740,6 +748,10 @@ export function collectDriftEntries(results: VitestJsonResult): CollectResult { expected: "(not in knownModels set)", real: id, mock: NO_MOCK_LEG, + // D6.2: set `id` to the model id so the delta layer (D6.1) + // can key by provider+id, yielding one distinct key per model + // rather than collapsing all entries under path:"knownModels". + id, })), ...(canary.truncated ? [ diff --git a/src/__tests__/drift-collector.test.ts b/src/__tests__/drift-collector.test.ts index 4eeabc47..59c5e1fb 100644 --- a/src/__tests__/drift-collector.test.ts +++ b/src/__tests__/drift-collector.test.ts @@ -1267,3 +1267,85 @@ describe("classifyUnparseableAsInfra", () => { } }); }); + +// --------------------------------------------------------------------------- +// D6.2 — per-item `id` field on ParsedDiff +// +// The delta layer (D6.1) keys findings by `provider+id`. For N distinct unknown +// model ids, the collector must produce N DISTINCT per-item `id` values so that +// a downstream `provider+id` keying yields N distinct keys — not 1 collapsed +// key under `path:"knownModels"` (pre-fix behaviour when `id` was absent/undefined). +// +// RED (pre-fix): `id` is unset on every ParsedDiff produced by the canary path, +// so all 3 diffs have `id === undefined` → only 1 distinct key. +// GREEN (post-fix): each diff carries `id` = the model id stored in `diff.real`, +// so 3 distinct unknown ids → 3 distinct `id` values → 3 distinct keys. +// --------------------------------------------------------------------------- + +describe("D6.2 — per-item id on ParsedDiff", () => { + // Three distinct hypothetical unknown model ids (not in the knownModels set + // in ws-realtime.drift.ts — A4 note: use future/hypothetical ids only). + const THREE_UNKNOWN_IDS_CANARY = + "AssertionError: UNKNOWN_REALTIME_MODELS=gpt-realtime-x1,gpt-realtime-x2,gpt-realtime-x3: " + + "expected [ 'gpt-realtime-x1', …(2) ] to deeply equal []\n" + + " at /repo/src/__tests__/drift/ws-realtime.drift.ts:108:69"; + + it("D6.2 RED→GREEN: 3 distinct canary model ids produce 3 DISTINCT per-item id fields (not collapsed under undefined)", () => { + const result = makeResult([ + makeAssertion({ + status: "failed", + ancestorTitles: ["OpenAI Realtime API drift"], + title: "canary: GA realtime models available", + failureMessages: [THREE_UNKNOWN_IDS_CANARY], + }), + ]); + + const entries = entriesOf(result); + expect(entries).toHaveLength(1); + const entry = entries[0]; + expect(entry.provider).toBe("OpenAI Realtime"); + + // There should be exactly 3 diffs — one per unknown model id. + expect(entry.diffs).toHaveLength(3); + + // D6.2 core assertion: each diff must carry a populated `id` field equal to + // the model id in `diff.real`, and all three must be distinct. + const ids = entry.diffs.map((d) => d.id); + expect(ids).toEqual(["gpt-realtime-x1", "gpt-realtime-x2", "gpt-realtime-x3"]); + + // All three ids are defined (not undefined). + expect(ids.every((id) => id !== undefined)).toBe(true); + + // All three ids are DISTINCT — a downstream provider+id key would yield 3 + // different keys, not 1 collapsed key under undefined/absent id. + const distinctIds = new Set(ids); + expect(distinctIds.size).toBe(3); + + // The model ids must match what's in `diff.real` (the source of truth). + for (const diff of entry.diffs) { + expect(diff.id).toBe(diff.real); + } + }); + + it("D6.2: parseDriftBlock-path diffs carry a stable id derived from path", () => { + // For regular drift-block diffs (not canary), `id` is derived from + // the `path` field so different paths → different ids. + const formatted = formatDriftReport("OpenAI Chat (non-streaming text)", [ + { ...SAMPLE_DIFF, path: "choices[0].message.refusal" }, + { ...SAMPLE_DIFF, path: "choices[0].message.content", severity: "warning" as const }, + ]); + const parsed = parseDriftBlock(formatted); + expect(parsed).not.toBeNull(); + expect(parsed!.diffs).toHaveLength(2); + + const ids = parsed!.diffs.map((d) => d.id); + // Both diffs must have a non-empty id. + expect(ids.every((id) => id !== undefined && id !== "")).toBe(true); + // The two paths are different → two distinct ids. + expect(new Set(ids).size).toBe(2); + // Each id must be derived from (or equal to) the path. + for (const diff of parsed!.diffs) { + expect(diff.id).toBe(diff.path); + } + }); +}); From 13c66dc83b273dbfcbb9a5bf26dc6803e6c89622 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 22:51:21 -0700 Subject: [PATCH 20/24] feat: add delta-gating core (computeDelta + isBaseReportReusable) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds scripts/drift-delta.ts: a pure, side-effect-free delta layer that reduces a base (main) report and head (PR) report to block/advisory/fixed keyed by provider+per-item-id. Routing is by key presence ONLY — DriftClass is annotation and never enters the block/advisory decision: - new-in-head (absent from base) -> block (diff-attributable) - present in both -> advisory (environmental/world drift) - base-only -> fixed (informational) isBaseReportReusable (O-2) guards cached-base reuse: non-empty entries + known-good conclusion + same-UTC-day. Includes the M-1 golden regression (#292): a real-drift/critical finding new-in-head MUST block regardless of class (the old class-routed rule would have greenlit it). --- scripts/drift-delta.ts | Bin 0 -> 5610 bytes src/__tests__/drift-delta.test.ts | 195 ++++++++++++++++++++++++++++++ 2 files changed, 195 insertions(+) create mode 100644 scripts/drift-delta.ts create mode 100644 src/__tests__/drift-delta.test.ts diff --git a/scripts/drift-delta.ts b/scripts/drift-delta.ts new file mode 100644 index 0000000000000000000000000000000000000000..3e4018fd65728ce2bde383ec60cc7c4e44843000 GIT binary patch literal 5610 zcma)A+in}l5zVu{qE=a8Nr#ujd5RpZLCN&4BbgFNdl!abgsqt-+17A|=^lzEWC3}~ z2L$=T{gRxj>L!P_Vno=5hh$H6)u~gbs=NQ>$q_xF7q+NPcW!D|&MB+BrCH^vURcV# zo7J>(D_gkI_OO`O59oT_4Oz9ad0gSwC9QmwuQMCS(9grc3-WeV`T7|?2D7x?w{On7 zx!F=?vIX9IJ$N-DyvFt#Z=MY%!}w;8-oOKRWofw%HC1J?rL)Vc^9wx2ed~p($zjX7 z_U>j~o14P2#kx}W-E2ndGPfQ!@0{JxZ~y!c9MZ5%;jPKH*xm+P@&>dtuB4VN@0_p7 zCB8HT_1WO2@Y>jfTGx99_VDYMC4He&(ws z-I!q8CzK=ZZWEIZEsW1;29J|fli#^e`R(3n!U2}guHO7C#>kYp>~q2}b?qk?ge84C z04W)rxlhP#)76Gic8-uF`sr#kO#YwX0c=}>d{t@KJGRA{m$3WE&Ayt>ZRP@2at@Hi zJ6qCl^ybxgc=?WQwv0|pXJ&;zY%Hsc$IjCiOy={#gfOL6=3VVFQ|veR$J%&PB04J> zHf34W;*~Owffx;c8IIW`m|?xLz*lQQWapVuthAf1E4z}SCuy@M1oVnRSv=_oxQ6ZB zRP~~pplU2fl=_B{fAHd$SCgypyU43Z!{0ys%Ts`OXS{>^CnN^J?4m0)@2Aaa=E|Ha zJQWRGOj*zhQ=)l02UiLk0#!507_U_L*K2440})&fGR$T+tGhFAt<+UnVGF#s4Ygf% z2p>yZqi}BJ8AT3p;CN8cY_NZHm+sD1W1k~l_q4zNy zKRY_=tAY*SQ8`DEF~iwO_=i-^9M}k2NVRfVQDL^$r*H=Q<$L7P4)6OZ{XvS- zAsm{z9IWB$o~}&2n09EhaM?n{hch_0rS&cYzq4CgQ{isGxZ~Th+LW&=lyo?rD)5_X z4JFB{qA)A4a)e^#lFDk<2`l9(VFu&|x=@vtU2FGfDyf+Aj+ybEL#r2H-+VzUgbXQS z)o#!hiDJ)6dk6hn84tcY_zs0eg-6{*RL8e3I`Gi z8!t^!1n3cD5RfqbKp&;A(iit|`IpwuweSTua3Rxbmijy_ZV*&JVo(5&*1;XxO4P`> zgBW^#(je-yqX*%j2$l#p;HWp2eN+8xaZ71OrPqRwx_v!PUyJ$2b1=4HUDUfYj2fyE zsUD>EzNPrldx;9pa~&YH??0p@ywsWrQ9032OA(~Q1|Zvfs|Unw5@!GRPN^%*nd+&l zagq)iL$oek;Lx;YfSG=_b|@___RN%3$?_igCXGSLBb9uQfB{m?{X)6ftag`kVbHHa zGHIu1pn)Y#b)bMoW@y#uv!UIAO^AA;r>jHM2nrRsS{0nleOZHhS@NJXiWJHW=q=Y) zjsoKOih-zkioky+wK5eMHp%T&`i8#)aoONu+uW%O_e_)d3JR=)nTUG-Xtsv*@Q*0H zt63|7kxHq90!+^sEVO!sj^5U5U()pZ`}EGk@4mlx`NK371^f51z!)Ocpt~ZeGVCnP zio45{3!cZC#L8(;L0^LPZUsRXh<9?yChV`f3>}+U@(Swx? zBLgmP^pv-86$HMov_^D-?AN0FEHOGRiTr2Y3+n!WSQNPDVA;N5Z*da zW`2_$Ihe0yQr?frz+zWxU(d(8300yFn2Xuo!A!hc0!t~N9!)TLGGpTjE3dLBP_4tE zIM!-xu{+7jG}TgzErwo8^`LHXa>6O=ksnad@?xVyE1@Nar%G6TuVGtAsOtpO!*%r zPCw8-Bdc{-w4#AjZ-n6&<2|x=nmN zxs$eu^?$D(fpc-zLpPU0>_bx&E_jab&Z|l%cN}{l!azSO)_fXe-NZ%2(qQOElhzv! z;ekp6)gCt(I11GZ97ZZ;x3}wnBkG{sqPz1f%^=}aYQmyJ`=e+F*B5e#Xw%Xf*$6Ej zAaE<<(v(uc^zZoULw@0#ny5}VnuP_`BF8O{K{7px-em62;@$@Cp^fo1k&GbHjCst#-5K%S zS!>Ery{)qsW;<=mY>3-6TvF{bukOJ^J!dXmNcTWYO&Gytl@L*R#q!wQW8l2e=xThTZQh{AKLsnHK$1#O1RH)|vK#1uxm1~UI zpXEXicNsQEE*eF+n@%5JxJT{;t)*hZc8(z8@mT~)6eV)+^Jn^Q;Jw-QT#$d`RJR|p zutL|SuX=?o=V-yFr>FGCq*765Fe(MhlMl}xvd^9{#Cttfcp_~3bvwm<%x3& = {}): ParsedDiff { + return { + path: "knownModels", + severity: "critical", + issue: "model drift", + expected: "x", + real: "y", + mock: "z", + ...overrides, + }; +} + +function report( + provider: string, + diffs: ParsedDiff[], + timestamp = "2026-07-14T00:00:00.000Z", +): DriftReport { + return { + timestamp, + entries: [ + { + provider, + scenario: "s", + builderFile: "b.ts", + builderFunctions: ["f"], + typesFile: null, + sdkShapesFile: "shapes.ts", + diffs, + }, + ], + }; +} + +function keys(list: DeltaKey[]): string[] { + return list.map((k) => `${k.provider}:${k.id}`).sort(); +} + +// --------------------------------------------------------------------------- +// The M-1 golden regression (#292). +// +// A real-drift/critical failure that is NEW-in-head MUST BLOCK. The old broken +// rule routed by CLASS (real-drift/critical → advisory), which would have +// greenlit #292. This test proves the class-routed rule fails and computeDelta +// blocks regardless of class. +// --------------------------------------------------------------------------- +describe("M-1 golden: new-in-head critical MUST block regardless of class", () => { + // Head introduces a critical/real-drift finding that base does not have. + const base = report("anthropic", [diff({ id: "claude-3-opus", class: DriftClass.None })]); + const head = report("anthropic", [ + diff({ id: "claude-3-opus", class: DriftClass.None }), + diff({ id: "claude-4-new-model", class: DriftClass.Critical }), + ]); + + // Simulate the OLD broken rule: route purely by class. A critical drift is + // sent to advisory, so a new-in-head #292 failure is NOT blocked. This stub + // encodes the pre-fix behavior we are regressing against. + const classRouted = (r: DriftReport) => { + const block: DeltaKey[] = []; + const advisory: DeltaKey[] = []; + for (const entry of r.entries) { + for (const d of entry.diffs) { + const dk: DeltaKey = { provider: entry.provider, id: d.id ?? d.path, class: d.class }; + if (d.class === DriftClass.Critical) advisory.push(dk); + else block.push(dk); + } + } + return { block, advisory }; + }; + + it("RED regression: the CLASS-ROUTED rule fails to block the new-in-head critical (would greenlight #292)", () => { + const result = classRouted(head); + const newCriticalBlocked = result.block.some((k) => k.id === "claude-4-new-model"); + // The broken rule sends the critical to advisory, NOT block. If this ever + // starts blocking, the class-routed regression is no longer being exercised. + expect(newCriticalBlocked).toBe(false); + expect(result.advisory.some((k) => k.id === "claude-4-new-model")).toBe(true); + }); + + it("GREEN: computeDelta blocks the new-in-head critical regardless of class", () => { + const { block, advisory } = computeDelta(base, head); + expect(keys(block)).toEqual(["anthropic:claude-4-new-model"]); + expect(keys(advisory)).toEqual(["anthropic:claude-3-opus"]); + // The blocked key is critical — proving class did not route it to advisory. + expect(block[0].class).toBe(DriftClass.Critical); + }); +}); + +describe("computeDelta routing by key presence", () => { + it("same key in base+head → advisory (even critical)", () => { + const base = report("openai", [diff({ id: "gpt-4", class: DriftClass.Critical })]); + const head = report("openai", [diff({ id: "gpt-4", class: DriftClass.Critical })]); + const { block, advisory, fixed } = computeDelta(base, head); + expect(block).toEqual([]); + expect(keys(advisory)).toEqual(["openai:gpt-4"]); + expect(fixed).toEqual([]); + }); + + it("head-only transient → block (keyed by provider+id)", () => { + const base = report("openai", []); + const head = report("openai", [diff({ id: "gpt-5-preview", class: DriftClass.Critical })]); + const { block, advisory, fixed } = computeDelta(base, head); + expect(keys(block)).toEqual(["openai:gpt-5-preview"]); + expect(advisory).toEqual([]); + expect(fixed).toEqual([]); + }); + + it("base-only failure → fixed (informational, not block/advisory)", () => { + const base = report("openai", [diff({ id: "gpt-3.5", class: DriftClass.Critical })]); + const head = report("openai", []); + const { block, advisory, fixed } = computeDelta(base, head); + expect(block).toEqual([]); + expect(advisory).toEqual([]); + expect(keys(fixed)).toEqual(["openai:gpt-3.5"]); + }); + + it("keys by provider+id (path bucket must NOT collapse N distinct ids into one)", () => { + const base = report("anthropic", []); + const head = report("anthropic", [ + diff({ path: "knownModels", id: "a" }), + diff({ path: "knownModels", id: "b" }), + diff({ path: "knownModels", id: "c" }), + ]); + const { block } = computeDelta(base, head); + expect(keys(block)).toEqual(["anthropic:a", "anthropic:b", "anthropic:c"]); + }); + + it("same id across different providers → distinct keys", () => { + const base = report("openai", [diff({ id: "shared" })]); + const head = { + timestamp: base.timestamp, + entries: [...base.entries, ...report("anthropic", [diff({ id: "shared" })]).entries], + }; + const { block, advisory } = computeDelta(base, head); + expect(keys(block)).toEqual(["anthropic:shared"]); + expect(keys(advisory)).toEqual(["openai:shared"]); + }); + + it("falls back to path when id is absent (legacy diffs still participate)", () => { + const base = report("cohere", []); + const head = report("cohere", [diff({ path: "legacyBucket" })]); // no id + const { block } = computeDelta(base, head); + expect(keys(block)).toEqual(["cohere:legacyBucket"]); + }); +}); + +describe("isBaseReportReusable (O-2)", () => { + const good = report("openai", [diff({ id: "gpt-4" })]); + + it("accepts a non-empty, known-good, same-UTC-day report", () => { + expect(isBaseReportReusable(good, "clean", true)).toBe(true); + expect(isBaseReportReusable(good, "success", true)).toBe(true); + }); + + it("rejects an empty-entries report (malformed cached base)", () => { + const empty: DriftReport = { timestamp: "2026-07-14T00:00:00.000Z", entries: [] }; + expect(isBaseReportReusable(empty, "clean", true)).toBe(false); + }); + + it("rejects a null / malformed report object", () => { + expect(isBaseReportReusable(null, "clean", true)).toBe(false); + expect(isBaseReportReusable(undefined, "clean", true)).toBe(false); + // Malformed: entries missing entirely. + expect(isBaseReportReusable({ timestamp: "t" } as unknown as DriftReport, "clean", true)).toBe( + false, + ); + }); + + it("rejects an unknown / bad conclusion (crash, quarantine, empty)", () => { + expect(isBaseReportReusable(good, "failure", true)).toBe(false); + expect(isBaseReportReusable(good, "quarantine", true)).toBe(false); + expect(isBaseReportReusable(good, "", true)).toBe(false); + expect(isBaseReportReusable(good, null, true)).toBe(false); + expect(isBaseReportReusable(good, undefined, true)).toBe(false); + }); + + it("rejects a stale (different-UTC-day) report", () => { + expect(isBaseReportReusable(good, "clean", false)).toBe(false); + }); +}); From b28c9ecce08c4db23daacbfd6ba29ad744cc721b Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 22:54:40 -0700 Subject: [PATCH 21/24] ci(drift): add drift-live-pr job skeleton (base+head two-run) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new drift-live-pr job to test-drift.yml that runs the live drift collector on both base (main) and head (the PR ref), producing two report artifacts. Base reuses the most recent same-UTC-day well-formed main scheduled report via isBaseReportReusable (D6.1), else runs a fresh live base. Triggered on pull_request (NOT pull_request_target, O3) so fork PRs run without secrets; path-filtered to drift scripts, drift tests, and drift workflows. This is the skeleton only — the delta comparison / block-vs- advisory step plugs in at the D6.3b SEAM marker. --- .github/workflows/test-drift.yml | 149 +++++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+) diff --git a/.github/workflows/test-drift.yml b/.github/workflows/test-drift.yml index c99dd2b3..9b96deaa 100644 --- a/.github/workflows/test-drift.yml +++ b/.github/workflows/test-drift.yml @@ -6,6 +6,11 @@ on: paths: - "src/agui-types.ts" - "src/__tests__/drift/agui-schema.drift.ts" + # PR-scoped live drift leg (drift-live-pr job): trigger when the drift + # scripts, any drift test, or a drift workflow itself changes. + - "scripts/drift-*.ts" + - "src/__tests__/drift/**" + - ".github/workflows/*drift*" workflow_dispatch: # Manual trigger permissions: @@ -311,3 +316,147 @@ jobs: curl -sf -X POST "$SLACK_WEBHOOK" \ -H "Content-Type: application/json" \ -d "$PAYLOAD" + + # PR-scoped live drift leg (D6.3a skeleton). Triggered ONLY on `pull_request` + # — deliberately NOT `pull_request_target` (O3): a `pull_request` run on a + # fork gets a read-only GITHUB_TOKEN and NO repo secrets, so untrusted fork + # code can never exfiltrate the provider keys. The trade-off (a fork PR has + # no live provider keys and so cannot run the live leg) is handled by the + # fork→maintainer-dry-run fallback that D6.3b documents; this skeleton only + # produces the two reports on the trusted (non-fork) path. + # + # The job runs the live drift collector on BOTH the base (`main`) and the + # head (the PR ref), emitting two artifacts — `drift-report-base` and + # `drift-report-head`. For base it first tries to REUSE the most recent + # same-UTC-day, well-formed `main` scheduled report (via + # `isBaseReportReusable`); only when no reusable report exists does it run a + # fresh live base. The delta comparison / block-vs-advisory decision that + # consumes these two reports is intentionally NOT here — that is D6.3b, which + # plugs in at the "D6.3b SEAM" marker below. + drift-live-pr: + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + timeout-minutes: 30 + env: + OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + GOOGLE_API_KEY: ${{ secrets.GOOGLE_API_KEY }} + steps: + # HEAD checkout: the PR ref (default checkout behavior on pull_request). + # persist-credentials:false — this leg never pushes; keeps the token off + # disk on the fork path. + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + persist-credentials: false + # Need main's history reachable so the base leg can check it out + # into a sibling worktree without a second full clone. + fetch-depth: 0 + - uses: pnpm/action-setup@0ebf47130e4866e96fce0953f49152a61190b271 # v6.0.9 + - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + with: + node-version: 24 + cache: pnpm + - run: pnpm install --frozen-lockfile + + # ---- BASE leg ------------------------------------------------------- + # Prefer reusing the most recent same-UTC-day, well-formed `main` + # scheduled drift report as the delta base — a same-day main baseline + # already captures the day's environmental drift, so reusing it avoids a + # redundant live run and keeps the PR fast. Only when no reusable report + # is available do we fall back to a fresh live base run. + # + # Reusability is decided by `isBaseReportReusable(report, conclusion, + # sameUtcDay)` from scripts/drift-delta.ts (D6.1): non-empty entries + + # known-good collector conclusion + same UTC day. The producing step + # writes `drift-report-base.json` either way, so downstream steps have a + # single stable base path regardless of which path produced it. + - name: Base drift report — reuse same-UTC-day main run, else run live + id: base + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + run: | + set -euo pipefail + # Attempt reuse: download the most recent successful scheduled + # `main` run's drift-report artifact, then let isBaseReportReusable + # (D6.1) decide if it is a trustworthy same-UTC-day baseline. On any + # failure to obtain a reusable report, fall through to a live run. + REUSED=false + if BASE_RUN_ID=$(gh run list \ + --workflow="Drift Tests" --repo="$REPO" --branch=main \ + --event=schedule --status=success --limit=1 \ + --json databaseId --jq '.[0].databaseId // empty'); then + if [ -n "$BASE_RUN_ID" ]; then + rm -rf ./.base-artifact + if gh run download "$BASE_RUN_ID" --repo="$REPO" \ + --name drift-report --dir ./.base-artifact; then + BASE_CONCLUSION=$(gh run view "$BASE_RUN_ID" --repo="$REPO" \ + --json conclusion --jq '.conclusion // empty' || echo "") + if node --input-type=module -e ' + import { readFileSync } from "node:fs"; + import { isBaseReportReusable } from "./scripts/drift-delta.ts"; + const path = "./.base-artifact/drift-report.json"; + let report = null; + try { report = JSON.parse(readFileSync(path, "utf-8")); } catch {} + const conclusion = report?.conclusion ?? process.env.BASE_CONCLUSION ?? ""; + const genAt = report?.generatedAt ? new Date(report.generatedAt) : null; + const now = new Date(); + const sameUtcDay = + !!genAt && + genAt.getUTCFullYear() === now.getUTCFullYear() && + genAt.getUTCMonth() === now.getUTCMonth() && + genAt.getUTCDate() === now.getUTCDate(); + process.exit(isBaseReportReusable(report, conclusion, sameUtcDay) ? 0 : 1); + ' BASE_CONCLUSION="$BASE_CONCLUSION"; then + cp ./.base-artifact/drift-report.json drift-report-base.json + REUSED=true + echo "base: reusing same-UTC-day main scheduled report (run $BASE_RUN_ID)" + fi + fi + fi + fi + + if [ "$REUSED" != "true" ]; then + echo "base: no reusable same-UTC-day main report — running fresh live base" + # Check main out into a sibling worktree and run the collector + # there, writing the base report back into the workspace root. + git worktree add ../base-main origin/main + ( cd ../base-main \ + && pnpm install --frozen-lockfile \ + && npx tsx scripts/drift-report-collector.ts \ + --out "$GITHUB_WORKSPACE/drift-report-base.json" ) + fi + echo "reused=$REUSED" >> "$GITHUB_OUTPUT" + + # ---- HEAD leg ------------------------------------------------------- + # Always run the collector live against the PR head (the checked-out + # ref) to produce the head report the delta compares against the base. + - name: Head drift report — run live on PR ref + run: npx tsx scripts/drift-report-collector.ts --out drift-report-head.json + + - name: Upload base drift report + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: drift-report-base + path: drift-report-base.json + if-no-files-found: warn + retention-days: 30 + + - name: Upload head drift report + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: drift-report-head + path: drift-report-head.json + if-no-files-found: warn + retention-days: 30 + + # =================================================================== + # D6.3b SEAM — the delta comparison / block-vs-advisory decision plugs + # in HERE. D6.3b adds a step that invokes `computeDelta(base, head)` from + # scripts/drift-delta.ts over drift-report-base.json + drift-report-head.json, + # blocks only on new-in-head keys, advisory-annotates base-present keys, + # and wires the fork→maintainer-dry-run fallback. This skeleton stops at + # producing the two reports above. + # =================================================================== From 1db651655170685d5e994f01373a66631621623a Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 22:58:08 -0700 Subject: [PATCH 22/24] =?UTF-8?q?ci(drift):=20wire=20delta=20gate=20?= =?UTF-8?q?=E2=80=94=20block=20new-in-head,=20advisory-annotate=20base-pre?= =?UTF-8?q?sent?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plug the D6.3b delta step into the drift-live-pr job at the SEAM. The step loads drift-report-base.json + drift-report-head.json and runs computeDelta (scripts/drift-delta.ts) via a quoted-heredoc tsx invocation, matching the preflight step's proven pattern. Routing is by KEY PRESENCE only (#292 invariant), never by DriftClass: - block[] (new-in-head) → ::error:: annotation + exit 1 (required check fails) - advisory[] (base-present) → ::notice:: neutral annotation + summary, exit 0 - fixed[] → informational summary line only Also documents the fork→maintainer-dry-run fallback in-job: fork PRs run without secrets, so a maintainer runs the delta dry-run from a trusted checkout and records the block/advisory conclusion. --- .github/workflows/test-drift.yml | 108 +++++++++++++++++++++++++++++-- 1 file changed, 102 insertions(+), 6 deletions(-) diff --git a/.github/workflows/test-drift.yml b/.github/workflows/test-drift.yml index 9b96deaa..6db0e5aa 100644 --- a/.github/workflows/test-drift.yml +++ b/.github/workflows/test-drift.yml @@ -453,10 +453,106 @@ jobs: retention-days: 30 # =================================================================== - # D6.3b SEAM — the delta comparison / block-vs-advisory decision plugs - # in HERE. D6.3b adds a step that invokes `computeDelta(base, head)` from - # scripts/drift-delta.ts over drift-report-base.json + drift-report-head.json, - # blocks only on new-in-head keys, advisory-annotates base-present keys, - # and wires the fork→maintainer-dry-run fallback. This skeleton stops at - # producing the two reports above. + # D6.3b — DELTA STEP. Load the two reports produced above and run + # `computeDelta(base, head)` (scripts/drift-delta.ts, D6.1). Routing is + # by KEY PRESENCE only (#292 invariant), NEVER by DriftClass: + # - block[] → keys NEW in head (absent from base): diff-attributable. + # ANY block key fails this required check (exit 1) — a + # new-in-head *critical* blocks, and so does a new-in-head + # advisory-class finding. Class is annotation only. + # - advisory[] → keys in BOTH base and head: pre-existing / world drift. + # Surfaced as a NEUTRAL annotation + job-summary line but + # NEVER fails the check (that is the daily job's concern, + # not the PR's). + # - fixed[] → keys in base but gone in head: informational only. + # The block-only-on-new-in-head decision is the incident-3 / M-1 backstop: + # a PR is blamed strictly for drift its diff introduced. + - name: Delta gate - block new-in-head, advisory-annotate base-present + run: | + set -euo pipefail + # Written at the checkout root (cwd) so the relative drift-delta + # import resolves against the repo — same rationale as the preflight + # step (ESM relative specifiers key off the module's own location). + # Quoted heredoc delimiter: no shell expansion inside the script. + DELTA_FILE="$GITHUB_WORKSPACE/.drift-delta-gate.mts" + trap 'rm -f "$DELTA_FILE"' EXIT + cat > "$DELTA_FILE" <<'DELTAGATE' + import { readFileSync, appendFileSync } from "node:fs"; + import { computeDelta } from "./scripts/drift-delta.js"; + import type { DeltaKey } from "./scripts/drift-delta.js"; + + const read = (p: string) => JSON.parse(readFileSync(p, "utf-8")); + const base = read("drift-report-base.json"); + const head = read("drift-report-head.json"); + + const { block, advisory, fixed } = computeDelta(base, head); + const fmt = (k: DeltaKey) => + `${k.provider} ${k.id}${k.class ? ` [${k.class}]` : ""}`; + + const summary: string[] = []; + summary.push("## Drift delta"); + summary.push(""); + summary.push(`- block (new-in-head): ${block.length}`); + summary.push(`- advisory (base-present): ${advisory.length}`); + summary.push(`- fixed (gone in head): ${fixed.length}`); + + // Advisory: pre-existing / environmental drift. Surface it as a + // NEUTRAL annotation (::notice::) and in the summary, but do NOT fail + // the check on it — that drift is the daily main job's concern. + for (const k of advisory) { + console.log(`::notice title=drift-advisory (pre-existing)::${fmt(k)}`); + } + if (advisory.length > 0) { + summary.push(""); + summary.push("### Advisory (pre-existing on main - not blamed on this PR)"); + for (const k of advisory) summary.push(`- ${fmt(k)}`); + } + + // Block: drift the diff INTRODUCED. Each is a loud error annotation; + // a non-empty block list fails the required check. + for (const k of block) { + console.log(`::error title=drift-block (new-in-head)::${fmt(k)}`); + } + if (block.length > 0) { + summary.push(""); + summary.push("### Block (introduced by this diff - required check FAILS)"); + for (const k of block) summary.push(`- ${fmt(k)}`); + } + + const summaryFile = process.env.GITHUB_STEP_SUMMARY; + if (summaryFile) { + appendFileSync(summaryFile, summary.join("\n") + "\n"); + } + + if (block.length > 0) { + console.error( + `::error::Drift gate BLOCKED: ${block.length} new-in-head failure(s) introduced by this diff: ${block.map(fmt).join(", ")}`, + ); + process.exit(1); + } + console.log( + advisory.length > 0 + ? `Drift gate PASSED: ${advisory.length} advisory (pre-existing) finding(s) surfaced, none new-in-head.` + : "Drift gate PASSED: no new-in-head drift.", + ); + DELTAGATE + npx tsx "$DELTA_FILE" + + # ---- fork → maintainer-dry-run fallback -------------------------------- + # This whole `drift-live-pr` job is gated on `pull_request` (NOT + # `pull_request_target`, O3), so a run originating from a FORK receives a + # read-only GITHUB_TOKEN and NO repo secrets — the provider API keys above + # are simply empty. Consequently a fork PR cannot run the live base/head + # collectors and this delta gate never executes meaningfully for it. + # + # Fallback protocol (maintainer dry-run): for a fork PR, a maintainer runs + # the delta gate manually from a trusted checkout — + # gh pr checkout # fetch the fork head + # npx tsx scripts/drift-report-collector.ts --out drift-report-head.json + # # (reuse or run a same-UTC-day main base as drift-report-base.json) + # node --input-type=module -e '' + # — and records the block/advisory conclusion as a PR comment. The + # branch-protection required-check registration for this job is a manual + # orchestrator step outside this diff (the check must be marked required in + # branch protection for the block=exit-1 signal to actually gate merges). # =================================================================== From 8618afe49252d49db7df1d37ed4a2a119f2a59f4 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 14 Jul 2026 23:05:48 -0700 Subject: [PATCH 23/24] fix(drift): use printable separator in drift-delta keyOf (was NUL, rendered binary) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the \x00 byte separator in keyOf() with '::' — a printable separator that cannot appear in a provider name or model id. The NUL caused git to classify scripts/drift-delta.ts as binary (Bin diff), making the file unreadable in any diff or review. Behavior-preserving: both base and head key through the same keyOf(), so equality/presence comparisons are unchanged. --- scripts/drift-delta.ts | Bin 5610 -> 5611 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/scripts/drift-delta.ts b/scripts/drift-delta.ts index 3e4018fd65728ce2bde383ec60cc7c4e44843000..4c435904cb07737a3864ec452fdb8f93097644c1 100644 GIT binary patch delta 15 WcmaE*{aSm2KNpjg)#d=MS=<0JFa=!z delta 14 VcmaE@{Yra-KNlmz=0L7l+yE>F1nB?( From 8c7fd8851c0a1ad6250fab9083e27bcc5527752f Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Wed, 15 Jul 2026 10:46:34 -0700 Subject: [PATCH 24/24] ci(drift): degrade drift-live-pr gracefully when provider keys absent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The drift-live-pr required check hard-failed on every drift-code PR: the head-leg step ran drift-report-collector.ts directly and let the collector's exit 2 ("critical drift found") propagate as a step failure, so the delta gate — the step that actually decides block-vs-advisory by key presence — was skipped and the check died for the wrong reason. A dead configured key also went silently green because the PR leg had no key-freshness preflight. Fix, scoped entirely to the workflow: - Add a PR-scoped per-provider key-triage preflight. No key configured for a provider -> SKIP that provider's live leg with a neutral ::notice:: advisory (not a failure; the drift suite's own describe.skipIf already drops the tests). Key present but 401/403 -> loud ::error title=stale-key:: and block (a configured-but-dead key stays a real failure, per the InfraError.status contract). Transient (429/5xx/network) warns and continues. Records live_legs_ran for downstream steps. - Base and head legs: capture the collector exit code and treat exit 2 (drift-present) as non-fatal — the report file is the signal, the delta gate interprets it. Exit 5 (quarantine) and any other non-zero still fail the leg. - When no provider key had a usable credential (fork PR / all keys absent), write empty well-formed base/head reports and neutral-pass the delta gate with a 'live drift leg skipped — no keys' advisory (maintainer-dry-run fallback applies). Delta blocking behavior is unchanged when keys exist: a new-in-head critical still fails the required check. --- .github/workflows/test-drift.yml | 197 +++++++++++++++++++++++++++++-- 1 file changed, 190 insertions(+), 7 deletions(-) diff --git a/.github/workflows/test-drift.yml b/.github/workflows/test-drift.yml index 6db0e5aa..5be8492c 100644 --- a/.github/workflows/test-drift.yml +++ b/.github/workflows/test-drift.yml @@ -358,6 +358,115 @@ jobs: cache: pnpm - run: pnpm install --frozen-lockfile + # ---- PR preflight — per-provider key triage (graceful degradation) --- + # The PR-scoped live leg must NOT hard-block a drift-code PR merely because + # a provider key is absent or a provider is transiently down — that would + # re-introduce the live-key coupling the whole delta design set out to + # reduce. So classify each provider independently, keying off the numeric + # InfraError.status (401/403), NEVER the prose message: + # + # - NO key configured → SKIP that provider's live leg with a + # NEUTRAL ::notice:: advisory. Not a failure. + # - key present but 401/403 → LOUD `stale-key` ::error:: → block. A + # configured key that is dead is a real + # failure (rotate it) — this is the one case + # that stays red (the InfraError.status + # contract, mirroring the daily preflight). + # - transient (429/5xx/net) → warn + continue; the collector's own retry + # path handles a transient blip. + # + # `live_legs_ran` records whether ANY provider had a usable key. When it is + # `false` (all skipped for no-key — e.g. a fork PR with no secrets), the + # delta gate below is a neutral pass with a "live drift leg skipped — no + # keys" advisory, and the maintainer-dry-run fallback (documented at the + # bottom of this job) applies. + - name: PR preflight — per-provider key triage + id: preflight + run: | + set -uo pipefail + # Written at the checkout root (cwd) so the relative provider import + # resolves against the repo — ESM relative specifiers key off the + # module's own location, not the process cwd. Cleaned up on exit. + PREFLIGHT_FILE="$GITHUB_WORKSPACE/.drift-pr-preflight.mts" + trap 'rm -f "$PREFLIGHT_FILE"' EXIT + cat > "$PREFLIGHT_FILE" <<'PREFLIGHT' + import { appendFileSync } from "node:fs"; + import { + InfraError, + listOpenAIModels, + listAnthropicModels, + listGeminiModels, + } from "./src/__tests__/drift/providers.js"; + + const PROVIDERS: { + name: string; + keyEnv: string; + list: (key: string) => Promise; + }[] = [ + { name: "OpenAI", keyEnv: "OPENAI_API_KEY", list: listOpenAIModels }, + { name: "Anthropic", keyEnv: "ANTHROPIC_API_KEY", list: listAnthropicModels }, + { name: "Gemini", keyEnv: "GOOGLE_API_KEY", list: listGeminiModels }, + ]; + + const stale: string[] = []; + let liveLegs = 0; + + for (const p of PROVIDERS) { + const key = process.env[p.keyEnv]; + if (!key) { + // NO key configured → SKIP this provider's live leg. Neutral + // advisory, NOT a failure (the drift suite's own describe.skipIf + // already drops the provider's tests when the key is absent). + console.log( + `::notice title=drift-live skipped (no key)::${p.name} live drift leg skipped — ${p.keyEnv} not configured`, + ); + continue; + } + try { + await p.list(key); + liveLegs++; + console.log(`preflight: ${p.name} key OK`); + } catch (err) { + const status = err instanceof InfraError ? err.status : 0; + if (status === 401 || status === 403) { + // stale-key: a CONFIGURED key is dead/expired/revoked. LOUD, + // classified, blocking — this stays a real failure. + console.error( + `::error title=stale-key::${p.name} preflight got HTTP ${status} — rotate ${p.keyEnv}`, + ); + stale.push(p.keyEnv); + } else { + // Transient (429/5xx/network) is NOT a stale key — don't block; + // count it as a live leg and let the collector's retry handle it. + liveLegs++; + const msg = err instanceof Error ? err.message : String(err); + console.warn( + `preflight: ${p.name} non-auth error (status ${status}), continuing: ${msg}`, + ); + } + } + } + + const out = process.env.GITHUB_OUTPUT; + if (out) { + appendFileSync(out, `live_legs_ran=${liveLegs > 0 ? "true" : "false"}\n`); + } + + if (stale.length > 0) { + console.error( + `::error::PR preflight failed (stale-key): rotate ${stale.join(", ")} — a configured key is dead`, + ); + process.exit(1); + } + if (liveLegs === 0) { + console.log( + "::notice title=drift-live skipped::no provider keys configured — live drift leg skipped (neutral); maintainer-dry-run fallback applies", + ); + } + console.log(`preflight: ${liveLegs} live provider leg(s) will run`); + PREFLIGHT + npx tsx "$PREFLIGHT_FILE" + # ---- BASE leg ------------------------------------------------------- # Prefer reusing the most recent same-UTC-day, well-formed `main` # scheduled drift report as the delta base — a same-day main baseline @@ -375,8 +484,19 @@ jobs: env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} REPO: ${{ github.repository }} + LIVE_LEGS_RAN: ${{ steps.preflight.outputs.live_legs_ran }} run: | set -euo pipefail + # No provider key had a usable credential (fork PR / all keys absent). + # The live legs are skipped — write an empty, well-formed base report so + # the delta gate has a stable file to read, and stop here (neutral). + if [ "${LIVE_LEGS_RAN:-true}" != "true" ]; then + echo "base: live legs skipped (no keys) — writing empty base report" + printf '%s\n' '{"timestamp":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","entries":[]}' \ + > drift-report-base.json + echo "reused=false" >> "$GITHUB_OUTPUT" + exit 0 + fi # Attempt reuse: download the most recent successful scheduled # `main` run's drift-report artifact, then let isBaseReportReusable # (D6.1) decide if it is a trustworthy same-UTC-day baseline. On any @@ -421,18 +541,64 @@ jobs: # Check main out into a sibling worktree and run the collector # there, writing the base report back into the workspace root. git worktree add ../base-main origin/main - ( cd ../base-main \ - && pnpm install --frozen-lockfile \ - && npx tsx scripts/drift-report-collector.ts \ - --out "$GITHUB_WORKSPACE/drift-report-base.json" ) + cd ../base-main + pnpm install --frozen-lockfile + # The collector exits 2 when it finds critical drift — that is the + # NORMAL "drift present" signal, NOT a step failure. The delta gate + # (not this leg) decides block-vs-advisory, so exit 2 here must be + # tolerated. Exit 5 (quarantine) and any other non-zero are genuine + # collector faults and DO fail the leg. `set +e` around the run so an + # exit 2 does not abort under `set -euo pipefail`. + set +e + npx tsx scripts/drift-report-collector.ts \ + --out "$GITHUB_WORKSPACE/drift-report-base.json" + BASE_EXIT=$? + set -e + cd "$GITHUB_WORKSPACE" + if [ "$BASE_EXIT" -ne 0 ] && [ "$BASE_EXIT" -ne 2 ]; then + echo "::error::Base collector faulted (exit $BASE_EXIT) — not a drift signal" + exit "$BASE_EXIT" + fi fi echo "reused=$REUSED" >> "$GITHUB_OUTPUT" # ---- HEAD leg ------------------------------------------------------- - # Always run the collector live against the PR head (the checked-out - # ref) to produce the head report the delta compares against the base. + # Run the collector live against the PR head (the checked-out ref) to + # produce the head report the delta compares against the base. + # + # CRITICAL (this was the wiring bug that hard-failed the required check): + # the collector exits 2 whenever it finds ANY critical drift — that is the + # EXPECTED "drift present" signal on the head leg, NOT a step failure. The + # whole point of the two-report + delta design is that `computeDelta` + # (the delta gate below) decides block-vs-advisory by KEY PRESENCE — a + # both-present critical is advisory, only a new-in-head key blocks. Letting + # the collector's exit 2 fail THIS step meant the delta gate was skipped + # and the check died for the wrong reason (every drift-code PR would + # red-fail regardless of whether its diff introduced the drift). So exit 2 + # here is tolerated; the report file is the signal. Exit 5 (quarantine) and + # any other non-zero are genuine collector faults and DO fail the leg. + # + # When no provider key had a usable credential (fork PR / all keys absent), + # skip the live head run and write an empty, well-formed head report so the + # delta gate has a stable file — it will then neutral-pass with an advisory. - name: Head drift report — run live on PR ref - run: npx tsx scripts/drift-report-collector.ts --out drift-report-head.json + env: + LIVE_LEGS_RAN: ${{ steps.preflight.outputs.live_legs_ran }} + run: | + set -uo pipefail + if [ "${LIVE_LEGS_RAN:-true}" != "true" ]; then + echo "head: live legs skipped (no keys) — writing empty head report" + printf '%s\n' '{"timestamp":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","entries":[]}' \ + > drift-report-head.json + exit 0 + fi + npx tsx scripts/drift-report-collector.ts --out drift-report-head.json + HEAD_EXIT=$? + if [ "$HEAD_EXIT" -ne 0 ] && [ "$HEAD_EXIT" -ne 2 ]; then + echo "::error::Head collector faulted (exit $HEAD_EXIT) — not a drift signal" + exit "$HEAD_EXIT" + fi + echo "head: collector exit $HEAD_EXIT (0 clean / 2 drift-present — both non-fatal here)" - name: Upload base drift report if: always() @@ -468,8 +634,25 @@ jobs: # The block-only-on-new-in-head decision is the incident-3 / M-1 backstop: # a PR is blamed strictly for drift its diff introduced. - name: Delta gate - block new-in-head, advisory-annotate base-present + env: + LIVE_LEGS_RAN: ${{ steps.preflight.outputs.live_legs_ran }} run: | set -euo pipefail + # When no provider key had a usable credential (fork PR / all keys + # absent), no live leg ran — there is no diff-attributable drift to + # evaluate. Neutral-pass with a "live drift leg skipped — no keys" + # advisory (the maintainer-dry-run fallback documented below applies), + # rather than blocking a drift-code PR on absent keys. + if [ "${LIVE_LEGS_RAN:-true}" != "true" ]; then + echo "::notice title=drift-live skipped (no keys)::live drift leg skipped — no provider keys configured; delta gate is a neutral pass (maintainer-dry-run fallback applies)" + { + echo "## Drift delta" + echo "" + echo "_Live drift leg skipped — no provider keys configured. Neutral pass; the maintainer-dry-run fallback applies._" + } >> "$GITHUB_STEP_SUMMARY" + echo "Drift gate PASSED (neutral): live legs skipped — no keys." + exit 0 + fi # Written at the checkout root (cwd) so the relative drift-delta # import resolves against the repo — same rationale as the preflight # step (ESM relative specifiers key off the module's own location).