diff --git a/README.md b/README.md index a10ce3d2..e4cd70fe 100644 --- a/README.md +++ b/README.md @@ -134,6 +134,40 @@ This ensures: - CI fails on new regressions (unexpected failures) - CI fails when a fix lands but the baseline isn't updated (stale entries) +### Baselining a single check + +A scenario is many checks — `server-stateless` alone is over twenty — so baselining the +whole scenario to excuse one of them stops enforcing the other nineteen. An entry can +instead name a single check, as `:`: + +```yaml +server: + - tasks-lifecycle # whole scenario may fail + - server-stateless:sep-2575-server-implements-discover # only this check may fail +``` + +The check id is the left-hand column the runner already prints for each check, so it can +be copied straight out of a failing run. + +With a per-check entry, every failing check in that scenario is judged on its own: the +named one is excused, and any other failure is still an unexpected regression. The four +exit-code rules above apply per check rather than per scenario, with one addition — a +baselined check that is absent or skipped is tolerated, because a scenario that bails on +a failed prerequisite legitimately never reaches its later checks, and the prerequisite +reports its own failure anyway. + +Two things to know: + +- **A check id addresses every occurrence of that id.** Ids repeat within a run (a loop, + a retried flow), and the occurrences collapse to one verdict, most-severe first. So + baselining a repeated id excuses all of its occurrences — coarser than ideal, still far + narrower than baselining the scenario. +- **Mind the space.** `- scenario:check-id` is a string; `- scenario: check-id` is YAML + for a mapping and is rejected with an error. + +A scenario cannot be listed both wholesale and per-check — the wholesale entry already +excuses everything, so the pair is contradictory and is rejected. + ## GitHub Action This repo provides a composite GitHub Action so SDK repos don't need to write their own conformance scripts. diff --git a/src/scenarios/client/auth/dpop.test.ts b/src/checks/collapse.test.ts similarity index 92% rename from src/scenarios/client/auth/dpop.test.ts rename to src/checks/collapse.test.ts index 5818e0d5..d590d468 100644 --- a/src/scenarios/client/auth/dpop.test.ts +++ b/src/checks/collapse.test.ts @@ -1,5 +1,5 @@ -import { collapseDuplicateChecks } from './dpop'; -import type { ConformanceCheck, CheckStatus } from '../../../types'; +import { collapseDuplicateChecks } from './collapse'; +import type { ConformanceCheck, CheckStatus } from '../types'; /** Minimal check factory for the dedupe unit tests. */ function chk(id: string, status: CheckStatus, tag?: string): ConformanceCheck { @@ -13,7 +13,7 @@ function chk(id: string, status: CheckStatus, tag?: string): ConformanceCheck { }; } -describe('collapseDuplicateChecks (DPoP nonce-posture shared-check dedupe)', () => { +describe('collapseDuplicateChecks', () => { it('collapses duplicate SUCCESS ids to a single entry', () => { const out = collapseDuplicateChecks([ chk('token-request', 'SUCCESS'), diff --git a/src/checks/collapse.ts b/src/checks/collapse.ts new file mode 100644 index 00000000..f308cc74 --- /dev/null +++ b/src/checks/collapse.ts @@ -0,0 +1,35 @@ +import type { CheckStatus, ConformanceCheck } from '../types'; + +/** + * Collapse duplicate non-INFO check IDs to a single entry, preferring the + * MOST-SEVERE occurrence (FAILURE > WARNING > SUCCESS > any other status, e.g. + * SKIPPED) so a real failure is never masked. Equal-severity ties keep the LAST + * occurrence. Per-request INFO log entries are always kept. + * + * A check ID is not unique within a run: scenarios re-emit a shared ID when a + * flow repeats. The RFC 9449 §8/§9 nonce round-trip re-POSTs /token (challenge + * → retry), so the shared token-flow checks (`token-request`, `pkce-*`) are + * appended twice; `sep-2575-http-server-meta-invalid-400` is emitted once per + * iteration of the `_meta` test-case loop. Collapsing reports each ID once + * without hiding a failure recorded on any occurrence, which is what lets an + * expected-failures baseline address a check by ID. + */ +export function collapseDuplicateChecks( + checks: ConformanceCheck[] +): ConformanceCheck[] { + const severity = (s: CheckStatus): number => + s === 'FAILURE' ? 3 : s === 'WARNING' ? 2 : s === 'SUCCESS' ? 1 : 0; + // Winning index per non-INFO id: highest severity, ties → last occurrence. + const winner = new Map(); + checks.forEach((c, i) => { + if (c.status === 'INFO') return; + const cur = winner.get(c.id); + if ( + cur === undefined || + severity(c.status) >= severity(checks[cur].status) + ) { + winner.set(c.id, i); + } + }); + return checks.filter((c, i) => c.status === 'INFO' || winner.get(c.id) === i); +} diff --git a/src/expected-failures.test.ts b/src/expected-failures.test.ts index 05cfc9c7..78bb55b8 100644 --- a/src/expected-failures.test.ts +++ b/src/expected-failures.test.ts @@ -2,9 +2,23 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { promises as fs } from 'fs'; import path from 'path'; import os from 'os'; -import { loadExpectedFailures, evaluateBaseline } from './expected-failures'; +import { + loadExpectedFailures, + evaluateBaseline, + type BaselineEntry +} from './expected-failures'; import { ConformanceCheck } from './types'; +/** A baseline entry allowing a whole scenario to fail. */ +function whole(scenario: string): BaselineEntry { + return { scenario }; +} + +/** A baseline entry allowing one check of a scenario to fail. */ +function only(scenario: string, checkId: string): BaselineEntry { + return { scenario, checkId }; +} + function makeCheck( id: string, status: 'SUCCESS' | 'FAILURE' | 'WARNING' | 'SKIPPED' | 'INFO' @@ -44,10 +58,13 @@ client: const result = await loadExpectedFailures(filePath); expect(result.server).toEqual([ - 'tools-call-with-progress', - 'resources-subscribe' + whole('tools-call-with-progress'), + whole('resources-subscribe') + ]); + expect(result.client).toEqual([ + whole('sse-retry'), + whole('auth/basic-dcr') ]); - expect(result.client).toEqual(['sse-retry', 'auth/basic-dcr']); }); it('loads a file with only server entries', async () => { @@ -60,7 +77,7 @@ client: ); const result = await loadExpectedFailures(filePath); - expect(result.server).toEqual(['tools-call-with-progress']); + expect(result.server).toEqual([whole('tools-call-with-progress')]); expect(result.client).toBeUndefined(); }); @@ -99,6 +116,73 @@ client: ); }); + it('names the mistake when a colon entry is written with a space', async () => { + // `- s: c` is a YAML mapping, not the string `- s:c`. Without a guard this + // stringifies to '[object Object]' and silently matches nothing. + const filePath = path.join(tmpDir, 'baseline.yml'); + await fs.writeFile( + filePath, + `server: + - server-stateless: sep-2575-server-implements-discover +` + ); + await expect(loadExpectedFailures(filePath)).rejects.toThrow( + /Remove the space after the colon: '- server-stateless:sep-2575-server-implements-discover'/ + ); + }); + + it('loads a per-check entry as a plain string', async () => { + const filePath = path.join(tmpDir, 'baseline.yml'); + await fs.writeFile( + filePath, + `server: + - server-stateless:sep-2575-server-implements-discover + - tasks-lifecycle +` + ); + const result = await loadExpectedFailures(filePath); + expect(result.server).toEqual([ + only('server-stateless', 'sep-2575-server-implements-discover'), + whole('tasks-lifecycle') + ]); + }); + + it('does not invent a check id for a trailing-colon entry', async () => { + // '- s:' parses to {s: null}. Advising '- s:null' would produce a check id + // named 'null' that matches nothing — the exact silent no-op this guard exists to stop. + const filePath = path.join(tmpDir, 'baseline.yml'); + await fs.writeFile(filePath, `server:\n - server-stateless:\n`); + await expect(loadExpectedFailures(filePath)).rejects.toThrow( + /takes no space after the colon/ + ); + await expect(loadExpectedFailures(filePath)).rejects.not.toThrow(/null'/); + }); + + it("rejects an empty list entry rather than coercing it to 'null'", async () => { + const filePath = path.join(tmpDir, 'baseline.yml'); + await fs.writeFile(filePath, `server:\n -\n`); + await expect(loadExpectedFailures(filePath)).rejects.toThrow( + /This one is empty/ + ); + }); + + it('rejects a scenario baselined both wholesale and per-check', async () => { + const filePath = path.join(tmpDir, 'baseline.yml'); + await fs.writeFile( + filePath, + `server:\n - server-stateless\n - server-stateless:some-check\n` + ); + await expect(loadExpectedFailures(filePath)).rejects.toThrow(/redundant/); + }); + + it('rejects an entry with an empty side of the colon', async () => { + const filePath = path.join(tmpDir, 'baseline.yml'); + await fs.writeFile(filePath, `server:\n - ':some-check'\n`); + await expect(loadExpectedFailures(filePath)).rejects.toThrow( + /Both sides of the colon are required/ + ); + }); + it('throws on missing file', async () => { await expect( loadExpectedFailures('/nonexistent/path.yml') @@ -119,7 +203,10 @@ describe('evaluateBaseline', () => { } ]; - const result = evaluateBaseline(results, ['scenario-a', 'scenario-b']); + const result = evaluateBaseline(results, [ + whole('scenario-a'), + whole('scenario-b') + ]); expect(result.exitCode).toBe(0); expect(result.expectedFailures).toEqual(['scenario-a', 'scenario-b']); expect(result.unexpectedFailures).toEqual([]); @@ -162,7 +249,7 @@ describe('evaluateBaseline', () => { } ]; - const result = evaluateBaseline(results, ['scenario-a']); + const result = evaluateBaseline(results, [whole('scenario-a')]); expect(result.exitCode).toBe(1); expect(result.staleEntries).toEqual(['scenario-a']); }); @@ -179,7 +266,7 @@ describe('evaluateBaseline', () => { } ]; - const result = evaluateBaseline(results, ['scenario-a']); + const result = evaluateBaseline(results, [whole('scenario-a')]); expect(result.exitCode).toBe(1); expect(result.staleEntries).toEqual(['scenario-a']); expect(result.unexpectedFailures).toEqual(['scenario-b']); @@ -199,7 +286,7 @@ describe('evaluateBaseline', () => { expect(result1.unexpectedFailures).toEqual(['scenario-a']); // In baseline → expected - const result2 = evaluateBaseline(results, ['scenario-a']); + const result2 = evaluateBaseline(results, [whole('scenario-a')]); expect(result2.exitCode).toBe(0); expect(result2.expectedFailures).toEqual(['scenario-a']); }); @@ -213,7 +300,7 @@ describe('evaluateBaseline', () => { ]; // scenario-z is in baseline but not in the results - should not be stale - const result = evaluateBaseline(results, ['scenario-z']); + const result = evaluateBaseline(results, [whole('scenario-z')]); expect(result.exitCode).toBe(0); expect(result.staleEntries).toEqual([]); }); @@ -234,7 +321,7 @@ describe('evaluateBaseline', () => { } ]; - const result = evaluateBaseline(results, ['expected-fail']); + const result = evaluateBaseline(results, [whole('expected-fail')]); expect(result.exitCode).toBe(1); expect(result.expectedFailures).toEqual(['expected-fail']); expect(result.unexpectedFailures).toEqual(['unexpected-fail']); @@ -254,8 +341,226 @@ describe('evaluateBaseline', () => { ]; // In baseline but passes (only SUCCESS/SKIPPED/INFO) → stale - const result = evaluateBaseline(results, ['scenario-a']); + const result = evaluateBaseline(results, [whole('scenario-a')]); expect(result.exitCode).toBe(1); expect(result.staleEntries).toEqual(['scenario-a']); }); }); + +describe('per-check baseline entries (scenario:check-id)', () => { + it('excuses only the named check, leaving its siblings enforced', () => { + const result = evaluateBaseline( + [ + { + scenario: 'server-stateless', + checks: [ + makeCheck('sep-2575-server-implements-discover', 'FAILURE'), + makeCheck('sep-2575-server-tags-subscription-id', 'SUCCESS') + ] + } + ], + [only('server-stateless', 'sep-2575-server-implements-discover')] + ); + expect(result.exitCode).toBe(0); + expect(result.expectedFailures).toEqual([ + 'server-stateless:sep-2575-server-implements-discover' + ]); + expect(result.unexpectedFailures).toEqual([]); + }); + + it('is the whole point: an unbaselined sibling failure still fails the run', () => { + const result = evaluateBaseline( + [ + { + scenario: 'server-stateless', + checks: [ + makeCheck('sep-2575-server-implements-discover', 'FAILURE'), + makeCheck('sep-2575-server-tags-subscription-id', 'FAILURE') + ] + } + ], + [only('server-stateless', 'sep-2575-server-implements-discover')] + ); + expect(result.exitCode).toBe(1); + expect(result.unexpectedFailures).toEqual([ + 'server-stateless:sep-2575-server-tags-subscription-id' + ]); + }); + + it('flags a baselined check that now passes as stale', () => { + const result = evaluateBaseline( + [ + { + scenario: 'server-stateless', + checks: [makeCheck('sep-2575-server-implements-discover', 'SUCCESS')] + } + ], + [only('server-stateless', 'sep-2575-server-implements-discover')] + ); + expect(result.exitCode).toBe(1); + expect(result.staleEntries).toEqual([ + 'server-stateless:sep-2575-server-implements-discover' + ]); + }); + + it('tolerates a baselined check that was not emitted (prerequisite bailed)', () => { + const result = evaluateBaseline( + [ + { + scenario: 'server-stateless', + checks: [makeCheck('other', 'SUCCESS')] + } + ], + [only('server-stateless', 'sep-2575-server-implements-discover')] + ); + expect(result.exitCode).toBe(0); + expect(result.staleEntries).toEqual([]); + }); + + it('treats a WARNING check as an expected failure, matching scenario-level semantics', () => { + const result = evaluateBaseline( + [ + { + scenario: 'server-stateless', + checks: [ + makeCheck('sep-2575-request-meta-client-info-optional', 'WARNING') + ] + } + ], + [only('server-stateless', 'sep-2575-request-meta-client-info-optional')] + ); + expect(result.exitCode).toBe(0); + expect(result.expectedFailures).toEqual([ + 'server-stateless:sep-2575-request-meta-client-info-optional' + ]); + }); + + it('excuses every occurrence of a repeated id, and a failure on any of them wins', () => { + // sep-2575-http-server-meta-invalid-400 is emitted once per _meta test case. + const result = evaluateBaseline( + [ + { + scenario: 'server-stateless', + checks: [ + makeCheck('sep-2575-http-server-meta-invalid-400', 'SUCCESS'), + makeCheck('sep-2575-http-server-meta-invalid-400', 'FAILURE'), + makeCheck('sep-2575-http-server-meta-invalid-400', 'SUCCESS') + ] + } + ], + [only('server-stateless', 'sep-2575-http-server-meta-invalid-400')] + ); + expect(result.exitCode).toBe(0); + expect(result.expectedFailures).toEqual([ + 'server-stateless:sep-2575-http-server-meta-invalid-400' + ]); + }); + + it('reports a repeated failing id once, not once per occurrence', () => { + const result = evaluateBaseline( + [ + { + scenario: 'server-stateless', + checks: [ + makeCheck('sep-2575-http-server-meta-invalid-400', 'FAILURE'), + makeCheck('sep-2575-http-server-meta-invalid-400', 'FAILURE') + ] + } + ], + [only('server-stateless', 'sep-2575-http-server-meta-invalid-400')] + ); + expect(result.expectedFailures).toEqual([ + 'server-stateless:sep-2575-http-server-meta-invalid-400' + ]); + }); + + it('judges staleness on the most-severe occurrence, not the first', () => { + // A SKIPPED occurrence ordered ahead of a SUCCESS must not mask the pass: + // the check demonstrably succeeded, so the baseline entry is stale. + const result = evaluateBaseline( + [ + { + scenario: 'server-stateless', + checks: [ + makeCheck('repeated', 'SKIPPED'), + makeCheck('repeated', 'SUCCESS') + ] + } + ], + [only('server-stateless', 'repeated')] + ); + expect(result.exitCode).toBe(1); + expect(result.staleEntries).toEqual(['server-stateless:repeated']); + }); + + it('does not let a repeated id hide a failure when it is NOT baselined', () => { + const result = evaluateBaseline( + [ + { + scenario: 'server-stateless', + checks: [ + makeCheck('repeated', 'SUCCESS'), + makeCheck('repeated', 'FAILURE') + ] + } + ], + [] + ); + expect(result.exitCode).toBe(1); + expect(result.unexpectedFailures).toEqual(['server-stateless']); + }); + + it('keeps whole-scenario entries working alongside per-check ones', () => { + const result = evaluateBaseline( + [ + { + scenario: 'tasks-lifecycle', + checks: [makeCheck('a', 'FAILURE'), makeCheck('b', 'FAILURE')] + }, + { + scenario: 'server-stateless', + checks: [makeCheck('c', 'FAILURE'), makeCheck('d', 'SUCCESS')] + } + ], + [whole('tasks-lifecycle'), only('server-stateless', 'c')] + ); + expect(result.exitCode).toBe(0); + expect(result.expectedFailures).toEqual([ + 'tasks-lifecycle', + 'server-stateless:c' + ]); + }); + + it('splits on the first colon so slashed scenario names survive', () => { + const result = evaluateBaseline( + [ + { + scenario: 'auth/basic-dcr', + checks: [makeCheck('wif-grant-type', 'FAILURE')] + } + ], + [only('auth/basic-dcr', 'wif-grant-type')] + ); + expect(result.exitCode).toBe(0); + expect(result.expectedFailures).toEqual(['auth/basic-dcr:wif-grant-type']); + }); + + it('judges staleness past an INFO entry sharing the id', () => { + // collapseDuplicateChecks keeps every INFO, so an INFO can sit ahead of the + // real verdict for that id. It reports nothing, and must not mask the pass. + const result = evaluateBaseline( + [ + { + scenario: 'server-stateless', + checks: [ + makeCheck('repeated', 'INFO'), + makeCheck('repeated', 'SUCCESS') + ] + } + ], + [only('server-stateless', 'repeated')] + ); + expect(result.exitCode).toBe(1); + expect(result.staleEntries).toEqual(['server-stateless:repeated']); + }); +}); diff --git a/src/expected-failures.ts b/src/expected-failures.ts index 8d0655f0..0f372f49 100644 --- a/src/expected-failures.ts +++ b/src/expected-failures.ts @@ -2,31 +2,118 @@ import { promises as fs } from 'fs'; import { parse as parseYaml } from 'yaml'; import { ConformanceCheck } from './types'; import { COLORS } from './runner/utils'; +import { collapseDuplicateChecks } from './checks/collapse'; + +/** + * One line of a baseline list, parsed. + * + * Written as `` to allow a whole scenario to fail, or + * `:` to allow only that check to. + */ +export interface BaselineEntry { + scenario: string; + /** When set, only this check may fail; the scenario's others stay enforced. */ + checkId?: string; +} export interface ExpectedFailures { - server?: string[]; - client?: string[]; + server?: BaselineEntry[]; + client?: BaselineEntry[]; } export interface BaselineResult { /** Exit code: 0 if only expected failures, 1 if unexpected failures or stale baseline */ exitCode: number; - /** Scenarios that failed unexpectedly (not in baseline) */ + /** Entries that failed unexpectedly (not in baseline), formatted as written */ unexpectedFailures: string[]; - /** Scenarios in baseline that now pass (stale entries) */ + /** Entries in baseline that now pass (stale entries), formatted as written */ staleEntries: string[]; - /** Scenarios that failed as expected */ + /** Entries that failed as expected, formatted as written */ expectedFailures: string[]; } +/** Render an entry back in the form a user writes it. */ +export function formatEntry(entry: BaselineEntry): string { + return entry.checkId ? `${entry.scenario}:${entry.checkId}` : entry.scenario; +} + +/** + * Parse and validate one baseline list. This is the only place that knows the + * entry grammar: everything downstream consumes `BaselineEntry`. + * + * Scenario names contain slashes but never colons (`auth/basic-dcr`), and no + * check id contains one, so the first colon is an unambiguous delimiter. + */ +function parseEntryList(entries: unknown[], section: string): BaselineEntry[] { + const reject = (entry: unknown, why: string): never => { + throw new Error( + `Invalid expected-failures file: '${section}' entry ${JSON.stringify(entry)} ` + + `is not '' or ':'. ${why}` + ); + }; + + const parsed = entries.map((entry): BaselineEntry => { + if (entry === null || entry === undefined) { + return reject(entry, 'This one is empty.'); + } + if (typeof entry === 'object') { + // A mapping means a space crept in after the colon. Only say so when the + // parse is actually consistent with that — one key, one non-empty scalar + // value. Reconstructing the user's line from anything else quotes a line + // they never wrote. + const pairs = Array.isArray(entry) + ? [] + : Object.entries(entry as Record); + const [key, value] = pairs[0] ?? []; + const isSpaceTypo = + pairs.length === 1 && + value !== null && + value !== undefined && + typeof value !== 'object' && + String(value) !== ''; + return reject( + entry, + isSpaceTypo + ? `Did you write '- ${key}: ${value}'? Remove the space after the colon: '- ${key}:${value}'` + : 'A per-check entry takes no space after the colon.' + ); + } + + const text = String(entry); + const sep = text.indexOf(':'); + if (sep === -1) { + return { scenario: text }; + } + const scenario = text.slice(0, sep); + const checkId = text.slice(sep + 1); + if (!scenario || !checkId) { + return reject(entry, 'Both sides of the colon are required.'); + } + return { scenario, checkId }; + }); + + const wholesale = new Set( + parsed.filter((e) => !e.checkId).map((e) => e.scenario) + ); + for (const entry of parsed) { + if (entry.checkId && wholesale.has(entry.scenario)) { + throw new Error( + `Invalid expected-failures file: '${entry.scenario}' expects the whole scenario to fail, ` + + `so '${formatEntry(entry)}' is redundant. Remove one or the other.` + ); + } + } + return parsed; +} + /** * Load and parse an expected-failures YAML file. * * Expected format: * ```yaml * server: - * - scenario-name-1 - * - scenario-name-2 + * - scenario-name-1 # whole scenario may fail + * - scenario-name-2:check-id # only this check may fail * client: * - scenario-name-3 * ``` @@ -49,22 +136,15 @@ export async function loadExpectedFailures( const result: ExpectedFailures = {}; - if (parsed.server !== undefined) { - if (!Array.isArray(parsed.server)) { + for (const section of ['server', 'client'] as const) { + const list = parsed[section]; + if (list === undefined) continue; + if (!Array.isArray(list)) { throw new Error( - `Invalid expected-failures file: 'server' must be an array of scenario names` + `Invalid expected-failures file: '${section}' must be an array of scenario names` ); } - result.server = parsed.server.map(String); - } - - if (parsed.client !== undefined) { - if (!Array.isArray(parsed.client)) { - throw new Error( - `Invalid expected-failures file: 'client' must be an array of scenario names` - ); - } - result.client = parsed.client.map(String); + result[section] = parseEntryList(list, section); } return result; @@ -73,42 +153,88 @@ export async function loadExpectedFailures( /** * Evaluate scenario results against an expected-failures baseline. * - * Rules: + * Whole-scenario entry (`{ scenario }`) — unchanged from the original behaviour: * - Scenario fails and IS in baseline → expected (ok) * - Scenario fails and is NOT in baseline → unexpected failure (exit 1) * - Scenario passes and IS in baseline → stale entry (exit 1, must update baseline) * - Scenario passes and is NOT in baseline → normal pass (ok) + * + * Per-check entry (`{ scenario, checkId }`) — every failing check in that + * scenario is judged on its own, so one baselined check no longer excuses the + * other twenty: + * - Check fails and IS in baseline → expected (ok) + * - Check fails and is NOT in baseline → unexpected failure (exit 1) + * - Check passes and IS in baseline → stale entry (exit 1) + * - Check is absent or SKIPPED → no signal, tolerated + * + * A check id addresses every occurrence of that id, not one: ids repeat within + * a run (a loop, a retried flow), so the occurrences are collapsed most-severe- + * first before matching. Baselining an id that repeats therefore excuses all of + * its occurrences — coarser than ideal, still far narrower than the scenario. + * + * Absent and SKIPPED are tolerated for the same reason the runner already + * treats them as green (AGENTS.md, "Check conventions"): neither reports a + * requirement violated. The cost is that an entry naming a check that no longer + * exists — renamed, deleted, or typo'd — sits unnoticed. Whole-scenario entries + * already carry that weakness for scenarios outside the selected suite. */ export function evaluateBaseline( results: { scenario: string; checks: ConformanceCheck[] }[], - expectedScenarios: string[] + expectedEntries: BaselineEntry[] ): BaselineResult { - const expectedSet = new Set(expectedScenarios); + const scenarios = new Set( + expectedEntries.filter((e) => !e.checkId).map((e) => e.scenario) + ); + const checksByScenario = new Map>(); + for (const entry of expectedEntries) { + if (!entry.checkId) continue; + const ids = checksByScenario.get(entry.scenario) ?? new Set(); + ids.add(entry.checkId); + checksByScenario.set(entry.scenario, ids); + } + const unexpectedFailures: string[] = []; const staleEntries: string[] = []; const expectedFailures: string[] = []; - const seenScenarios = new Set(); + const isFailed = (c: ConformanceCheck) => + c.status === 'FAILURE' || c.status === 'WARNING'; for (const result of results) { - seenScenarios.add(result.scenario); - const hasFailed = - result.checks.some((c) => c.status === 'FAILURE') || - result.checks.some((c) => c.status === 'WARNING'); - const isExpected = expectedSet.has(result.scenario); - - if (hasFailed && isExpected) { - expectedFailures.push(result.scenario); - } else if (hasFailed && !isExpected) { - unexpectedFailures.push(result.scenario); - } else if (!hasFailed && isExpected) { - staleEntries.push(result.scenario); + if (scenarios.has(result.scenario)) { + const hasFailed = result.checks.some(isFailed); + if (hasFailed) expectedFailures.push(result.scenario); + else staleEntries.push(result.scenario); + continue; + } + + const expectedChecks = checksByScenario.get(result.scenario); + if (!expectedChecks) { + if (result.checks.some(isFailed)) + unexpectedFailures.push(result.scenario); + continue; } - // !hasFailed && !isExpected → normal pass, nothing to do - } - // Also check for baseline entries that reference scenarios not in the run - // (these are not stale - they might just not be in this suite) + const collapsed = collapseDuplicateChecks(result.checks); + for (const c of collapsed.filter(isFailed)) { + const entry = formatEntry({ scenario: result.scenario, checkId: c.id }); + (expectedChecks.has(c.id) ? expectedFailures : unexpectedFailures).push( + entry + ); + } + for (const id of expectedChecks) { + // Only a demonstrated pass makes the entry stale. Absent and SKIPPED + // report no requirement violated, so they leave the entry alone; a + // failure was already recorded above. INFO entries are kept rather than + // collapsed, so one can sit ahead of the real verdict for this id. + const emitted = collapsed.find((c) => c.id === id && c.status !== 'INFO'); + if (emitted?.status === 'SUCCESS') { + staleEntries.push( + formatEntry({ scenario: result.scenario, checkId: id }) + ); + } + } + } const exitCode = unexpectedFailures.length > 0 || staleEntries.length > 0 ? 1 : 0; @@ -124,8 +250,8 @@ export function printBaselineResults(result: BaselineResult): void { console.log( `\n${COLORS.YELLOW}Expected failures (in baseline):${COLORS.RESET}` ); - for (const scenario of result.expectedFailures) { - console.log(` ~ ${scenario}`); + for (const entry of result.expectedFailures) { + console.log(` ~ ${entry}`); } } @@ -133,8 +259,8 @@ export function printBaselineResults(result: BaselineResult): void { console.log( `\n${COLORS.RED}Stale baseline entries (now passing - remove from baseline):${COLORS.RESET}` ); - for (const scenario of result.staleEntries) { - console.log(` ✓ ${scenario}`); + for (const entry of result.staleEntries) { + console.log(` ✓ ${entry}`); } } @@ -142,8 +268,8 @@ export function printBaselineResults(result: BaselineResult): void { console.log( `\n${COLORS.RED}Unexpected failures (not in baseline):${COLORS.RESET}` ); - for (const scenario of result.unexpectedFailures) { - console.log(` ✗ ${scenario}`); + for (const entry of result.unexpectedFailures) { + console.log(` ✗ ${entry}`); } } @@ -154,12 +280,12 @@ export function printBaselineResults(result: BaselineResult): void { } else { if (result.staleEntries.length > 0) { console.log( - `\n${COLORS.RED}Baseline is stale: update your expected-failures file to remove passing scenarios.${COLORS.RESET}` + `\n${COLORS.RED}Baseline is stale: update your expected-failures file to remove passing entries.${COLORS.RESET}` ); } if (result.unexpectedFailures.length > 0) { console.log( - `\n${COLORS.RED}Unexpected failures detected: these scenarios are not in your expected-failures baseline.${COLORS.RESET}` + `\n${COLORS.RED}Unexpected failures detected: these are not in your expected-failures baseline.${COLORS.RESET}` ); } } diff --git a/src/scenarios/client/auth/dpop.ts b/src/scenarios/client/auth/dpop.ts index e993369d..8b16ba5a 100644 --- a/src/scenarios/client/auth/dpop.ts +++ b/src/scenarios/client/auth/dpop.ts @@ -18,6 +18,7 @@ import { type DpopClientObservations } from './helpers/dpopResourceAuth'; import { SpecReferences } from './spec-references'; +import { collapseDuplicateChecks } from '../../../checks/collapse'; const PRM_PATH = '/.well-known/oauth-protected-resource/mcp'; @@ -111,38 +112,6 @@ function newTokenReqObs(): DpopTokenRequestObservation { }; } -/** - * Collapse duplicate non-INFO check IDs to a single entry, preferring the - * MOST-SEVERE occurrence (FAILURE > WARNING > SUCCESS > any other status, e.g. - * SKIPPED) so a real failure is never masked. Equal-severity ties keep the LAST - * occurrence (for the nonce round-trip that is the retry's diagnostic, which is - * status-equivalent to the first). Per-request INFO log entries are always kept. - * - * The RFC 9449 §8/§9 nonce round-trip re-POSTs /token (challenge → retry), so - * the shared token-flow conformance checks (`token-request`, `pkce-*`) are - * appended twice; this reports each once without hiding a failure recorded on - * either attempt. Exported so the behaviour is unit-tested directly. - */ -export function collapseDuplicateChecks( - checks: ConformanceCheck[] -): ConformanceCheck[] { - const severity = (s: CheckStatus): number => - s === 'FAILURE' ? 3 : s === 'WARNING' ? 2 : s === 'SUCCESS' ? 1 : 0; - // Winning index per non-INFO id: highest severity, ties → last occurrence. - const winner = new Map(); - checks.forEach((c, i) => { - if (c.status === 'INFO') return; - const cur = winner.get(c.id); - if ( - cur === undefined || - severity(c.status) >= severity(checks[cur].status) - ) { - winner.set(c.id, i); - } - }); - return checks.filter((c, i) => c.status === 'INFO' || winner.get(c.id) === i); -} - export class DPoPClientScenario implements Scenario { readonly name: string; readonly source = {