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 diff --git a/.github/workflows/test-drift.yml b/.github/workflows/test-drift.yml index 18b099be..5be8492c 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: @@ -44,6 +49,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 } @@ -54,6 +64,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 @@ -76,6 +169,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" @@ -129,6 +225,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: | @@ -137,9 +234,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 @@ -175,8 +280,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 @@ -204,3 +316,426 @@ 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 + + # ---- 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 + # 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 }} + 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 + # 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 + # 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 ------------------------------------------------------- + # 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 + 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() + 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 — 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 + 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). + # 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). + # =================================================================== diff --git a/scripts/drift-delta.ts b/scripts/drift-delta.ts new file mode 100644 index 00000000..4c435904 --- /dev/null +++ b/scripts/drift-delta.ts @@ -0,0 +1,146 @@ +/** + * Delta-gating core for the drift pipeline. + * + * A PR-scoped drift run produces a HEAD report; a same-UTC-day cached MAIN run + * produces a BASE report. The gate must only BLOCK on drift that is attributable + * to the diff under review — a drift already present on main (environmental / + * world drift) is the daily job's concern, NOT the PR's. This module reduces the + * two reports to a per-key delta: + * + * - block → keys NEW in head (absent from base): diff-attributable, hard fail + * - advisory → keys present in BOTH base and head: pre-existing / environmental + * - fixed → keys present in base but GONE in head: informational + * + * The block/advisory/fixed decision is driven ENTIRELY by key presence. The + * coarse `DriftClass` (critical/advisory/quarantine) is annotation only and NEVER + * enters the routing decision — a new-in-head *critical* still BLOCKS, and a + * both-present *critical* is still only ADVISORY. This is the #292 invariant: a + * real-drift/critical finding that is new in head must block regardless of class. + * + * Pure and side-effect-free — no filesystem, no network, no process state. + */ + +import type { DriftClass, DriftReport } from "./drift-types.js"; + +/** + * A single failure identified by the delta layer. + * + * A failure is keyed by `provider` + per-item `id` (NOT `path`, which is a + * generic bucket like `"knownModels"` that would collapse N distinct model + * drifts into one key). `class` is carried through purely for annotation and + * never participates in routing. + */ +export interface DeltaKey { + /** Provider the failing entry belongs to (e.g. "anthropic"). */ + provider: string; + /** Stable per-item key (e.g. a model id). Falls back to path when absent. */ + id: string; + /** Coarse classification — annotation only, never routes block vs advisory. */ + class?: DriftClass; +} + +/** The delta between a base (main) report and a head (PR) report. */ +export interface DeltaResult { + /** New-in-head failures — diff-attributable, hard fail. */ + block: DeltaKey[]; + /** Failures present in both base and head — pre-existing / environmental. */ + advisory: DeltaKey[]; + /** Failures present in base but gone in head — informational. */ + fixed: DeltaKey[]; +} + +/** + * Build the canonical string key for a failure. Keys by provider + per-item id. + * When a diff has no `id` (legacy diffs), fall back to its `path` so it still + * participates in the delta rather than being dropped — this is a coarse bucket + * but preserves the block/advisory distinction across reports. + */ +function keyOf(provider: string, id: string): string { + return `${provider}::${id}`; +} + +/** + * Flatten a report into a map of `key → DeltaKey`. When multiple diffs collapse + * to the same key, the last one wins for annotation purposes (the routing + * decision does not depend on which representative is retained). + */ +function indexReport(report: DriftReport): Map { + const index = new Map(); + for (const entry of report.entries) { + for (const diff of entry.diffs) { + // `id` is the stable per-item key; `path` is the legacy fallback bucket. + const id = diff.id ?? diff.path; + const key = keyOf(entry.provider, id); + index.set(key, { provider: entry.provider, id, class: diff.class }); + } + } + return index; +} + +/** + * Compute the delta between a base report and a head report. + * + * Routing is by KEY PRESENCE only: + * - key in head but not base → block (new-in-head, diff-attributable) + * - key in both → advisory (pre-existing / environmental) + * - key in base but not head → fixed (informational) + * + * `DriftClass` NEVER enters this decision — a new-in-head critical blocks, a + * both-present critical is advisory. See the module docstring (#292 invariant). + */ +export function computeDelta(baseReport: DriftReport, headReport: DriftReport): DeltaResult { + const base = indexReport(baseReport); + const head = indexReport(headReport); + + const block: DeltaKey[] = []; + const advisory: DeltaKey[] = []; + const fixed: DeltaKey[] = []; + + for (const [key, headKey] of head) { + if (base.has(key)) { + advisory.push(headKey); + } else { + block.push(headKey); + } + } + + for (const [key, baseKey] of base) { + if (!head.has(key)) { + fixed.push(baseKey); + } + } + + return { block, advisory, fixed }; +} + +/** + * Known-good collector conclusions that make a cached base report trustworthy to + * reuse. Anything else (crash, quarantine, unknown) means the base is not a + * reliable baseline and a fresh live base run should be preferred. + */ +const REUSABLE_CONCLUSIONS: ReadonlySet = new Set(["clean", "success"]); + +/** + * Guard (O-2) for reusing a cached same-UTC-day main report as the delta base. + * + * A base report is reusable only when ALL of: + * 1. it has a non-empty `entries[]` (rejects empty / malformed cached JSON), + * 2. its collector `conclusion` is known-good (rejects crash / quarantine), + * 3. it was produced on the same UTC day (`sameUtcDay`) — a stale baseline + * would misattribute a day's worth of environmental drift to the PR. + * + * Anything else → not reusable → caller should run a fresh live base. + */ +export function isBaseReportReusable( + report: DriftReport | null | undefined, + conclusion: string | null | undefined, + sameUtcDay: boolean, +): boolean { + if (!report || !Array.isArray(report.entries) || report.entries.length === 0) { + return false; + } + if (!conclusion || !REUSABLE_CONCLUSIONS.has(conclusion)) { + return false; + } + return sameUtcDay === true; +} diff --git a/scripts/drift-report-collector.ts b/scripts/drift-report-collector.ts index 673fb7fb..1108a8c4 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] @@ -21,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) @@ -200,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, }); } @@ -596,6 +607,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 @@ -617,9 +656,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) { @@ -678,6 +730,10 @@ export function collectDriftEntries(results: VitestJsonResult): DriftEntry[] { 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 @@ -692,6 +748,10 @@ export function collectDriftEntries(results: VitestJsonResult): DriftEntry[] { 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 ? [ @@ -730,15 +790,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; } @@ -754,26 +829,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)}`); } @@ -784,13 +871,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( @@ -798,7 +894,7 @@ export function collectDriftEntries(results: VitestJsonResult): DriftEntry[] { ); } - return entries; + return { entries, quarantine }; } // --------------------------------------------------------------------------- @@ -984,6 +1080,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 // --------------------------------------------------------------------------- @@ -999,7 +1128,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..."); @@ -1014,10 +1144,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 { @@ -1042,17 +1174,26 @@ function main(): void { ); console.log(` Critical diffs: ${criticalCount}`); - if (criticalCount > 0) { - console.log("Exiting with code 2 (critical diffs found)."); - process.exit(2); + const quarantineCount = quarantine.length; + console.log(` Quarantined failures: ${quarantineCount}`); + + 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."); } /** 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/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/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[]; } diff --git a/src/__tests__/drift-collector.test.ts b/src/__tests__/drift-collector.test.ts index 6bf2d672..59c5e1fb 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,127 @@ 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); + }); + } + }); +}); + +// --------------------------------------------------------------------------- +// 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); } }); }); diff --git a/src/__tests__/drift-delta.test.ts b/src/__tests__/drift-delta.test.ts new file mode 100644 index 00000000..833e08a3 --- /dev/null +++ b/src/__tests__/drift-delta.test.ts @@ -0,0 +1,195 @@ +import { describe, it, expect } from "vitest"; + +import { computeDelta, isBaseReportReusable } from "../../scripts/drift-delta.js"; +import type { DeltaKey } from "../../scripts/drift-delta.js"; +import { DriftClass } from "../../scripts/drift-types.js"; +import type { DriftReport, ParsedDiff } from "../../scripts/drift-types.js"; + +// --------------------------------------------------------------------------- +// drift-delta: the delta-gating core. +// +// The gate must BLOCK only on drift attributable to the PR diff (new-in-head), +// treat drift already present on main as ADVISORY (environmental / world drift), +// and report base-only drift as FIXED. The block/advisory decision is by KEY +// PRESENCE alone — `DriftClass` is annotation and must NEVER route. +// --------------------------------------------------------------------------- + +function diff(overrides: Partial = {}): 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); + }); +}); 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); + }); }); 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"); + }); +}); diff --git a/src/__tests__/drift/model-family.test.ts b/src/__tests__/drift/model-family.test.ts new file mode 100644 index 00000000..da230e95 --- /dev/null +++ b/src/__tests__/drift/model-family.test.ts @@ -0,0 +1,35 @@ +/** + * 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"); + }); + + 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 new file mode 100644 index 00000000..6489f64a --- /dev/null +++ b/src/__tests__/drift/model-family.ts @@ -0,0 +1,59 @@ +/** + * 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 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`) + * + * 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 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 { + let family = id; + for (;;) { + 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; + } + return family; +} diff --git a/src/__tests__/drift/model-registry.test.ts b/src/__tests__/drift/model-registry.test.ts new file mode 100644 index 00000000..b6e02703 --- /dev/null +++ b/src/__tests__/drift/model-registry.test.ts @@ -0,0 +1,189 @@ +/** + * Unit test for the per-provider model-family registry shape, plus the §6.1c + * builder/fixture cross-check. + * + * 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( + 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); + } + } + }); +}); + +// ─── 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/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"]); 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"); - }); -}); diff --git a/src/__tests__/drift/models.drift.ts b/src/__tests__/drift/models.drift.ts index 1e5fa556..d2737a37 100644 --- a/src/__tests__/drift/models.drift.ts +++ b/src/__tests__/drift/models.drift.ts @@ -1,130 +1,185 @@ /** - * 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", -]; - -// 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; - -// 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"]); - -// 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), - ); +/** + * 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([]); } // --------------------------------------------------------------------------- -// OpenAI +// 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. // --------------------------------------------------------------------------- -describe.skipIf(!process.env.OPENAI_API_KEY)("OpenAI model availability", () => { - it("models used in aimock tests are still available", 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); +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([]); + + // 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([]); + }); - if (referenced.length === 0) return; // no models found to check + 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([]); + }); - 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); - } + 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"]); }); }); // --------------------------------------------------------------------------- -// Anthropic +// OpenAI // --------------------------------------------------------------------------- -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.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!); + assertNoUnclassifiedFamilies(models, "openai", "OpenAI Chat (live /models family canary)"); }); }); // --------------------------------------------------------------------------- -// Gemini +// Anthropic // --------------------------------------------------------------------------- -describe.skipIf(!process.env.GOOGLE_API_KEY)("Gemini model availability", () => { - it("models used in aimock tests are still available", async () => { - const models = await listGeminiModels(process.env.GOOGLE_API_KEY!); - const referenced = scrapeModels(GEMINI_MODEL_PATTERN, sourceFiles); +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)", + ); + }); + }, +); - if (referenced.length === 0) return; - - // Drop experimental/live ids, markdown anchor fragments, and aimock - // provider-mode names (see filterStableGeminiModels). - const stable = filterStableGeminiModels(referenced); +// --------------------------------------------------------------------------- +// Gemini +// --------------------------------------------------------------------------- - 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); - } +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!); + assertNoUnclassifiedFamilies(models, "gemini", "Google Gemini (live /models family canary)"); }); }); 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); }); } 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);