From d8791f6724ee724826f6d4ef6b1175663a6ac676 Mon Sep 17 00:00:00 2001 From: Nate Otto Date: Wed, 14 Oct 2026 16:24:10 -0700 Subject: [PATCH 1/2] feat(results): require CheckResult.id and rename SuiteSummary.verified MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `runSuites` now computes `..` for every row it writes — normal, not-run, and both kinds of synthetic `.applies` row — so `CheckResult.id` is required and the verifier's after-the-fact `populateCheckIds` pass is gone. The parse-failure row gets its `cryptographic.parsing.envelope` id where it is built, and the summary code's fallback for a missing id goes with it. `computeId` moves to its own module so the suite runner can use it without importing the summarizer. `SuiteSummary.verified` becomes `hasFailures`, true when any check in the suite failed. "Verified" now exists only at the top level, where it means "no fatal check failed" — a non-fatal failure no longer reads as `verified: false` inside a `verified: true` result. `CheckResult.suite` is no longer deprecated: it is how rows are grouped, it matches `SuiteSummary.suite`, and it cannot be recovered from `id`, because suite ids contain dots. `check` stays deprecated. Docs: record that check ids and the built-in suites' phase tags are fixed for 2.x, and complete the API guide's list of ids with the status list-issuer check, the parsing suite and the Open Badges suites' synthetic `.applies` rows. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 23 ++++-- README.md | 10 ++- docs/api/timing.md | 2 +- docs/api/verification-results.md | 88 ++++++++++++++++------- docs/architecture.md | 2 +- src/compute-id.ts | 51 +++++++++++++ src/fold-results.ts | 46 ++---------- src/run-suites.ts | 31 +++++--- src/types/check.ts | 20 +++--- src/types/suite-summary.ts | 13 +++- src/verifier.ts | 30 ++------ test/flatten-presentation-results.spec.ts | 1 + test/fold-results.spec.ts | 33 +++++++-- test/run-suites.spec.ts | 82 +++++++++++++++++++++ test/types.spec.ts | 11 +-- test/verify-credential.spec.ts | 4 +- test/verify-presentation.spec.ts | 14 ++-- 17 files changed, 316 insertions(+), 145 deletions(-) create mode 100644 src/compute-id.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index bac23a1..78548ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,9 +14,18 @@ not run after a fatal failure. `@interop/ecdsa-multikey`. `defaultCryptoSuites()` now registers three suites: Ed25519Signature2020, EdDSA RDFC-2022, and ECDSA RDFC-2019. - `SuiteSummary` type and `summary: SuiteSummary[]` field on - `CredentialVerificationResult` and `PresentationVerificationResult`. -- `id: string` field on `CheckResult` — dot-separated - `..` namespace. + `CredentialVerificationResult` and `PresentationVerificationResult`. Each + entry carries `hasFailures: boolean` — true when any check in that suite + failed. It is not called `verified`: only the top-level `verified` is a + verdict ("no fatal check failed"), and a non-fatal failure sets + `hasFailures` on a suite inside a result that is still `verified: true`. +- Required `id: string` field on `CheckResult` — dot-separated + `..` namespace, set by the suite runner on every + row it emits (including not-run rows and synthetic `.applies` + rows), and by the parse-failure path on its one row. A suite with no + `phase` produces ids beginning `unknown.`. Check ids and the built-in + suites' phase tags are fixed for 2.x; since an id embeds its suite's + phase, changing either is a breaking change. - Pure `foldCheckResults` helper and `computeId` namespace builder, both exported from the package barrel. - New consumer doc at `docs/api/verification-results.md` covering the result @@ -224,8 +233,10 @@ not run after a fatal failure. ### Deprecated -- `CheckResult.check` and `CheckResult.suite` — use `CheckResult.id` instead. - Removal target: the next major. +- `CheckResult.check` — use `CheckResult.id` instead. Removal target: the + next major. `CheckResult.suite` is **not** deprecated: it is how rows are + grouped, it matches `SuiteSummary.suite`, and it cannot be recovered from + `id`, because a suite id may itself contain dots. ### Removed @@ -240,7 +251,7 @@ not run after a fatal failure. - To adopt the new shape: read `result.summary[]` for the per-suite rollup; read `result.results[]` for per-check detail; use - `r.id?.startsWith(summary.id + '.')` to find the detail rows under a summary + `r.id.startsWith(summary.id + '.')` to find the detail rows under a summary entry, narrowing by `r.outcome.status` when you want only the failures. - **Registry matches.** 1.x returned the registry's record of the issuer at the top level of the verification result, as `matchingIssuers[]`, each diff --git a/README.md b/README.md index 2928eed..c9c38e6 100644 --- a/README.md +++ b/README.md @@ -811,19 +811,23 @@ a `summary: SuiteSummary[]` rollup of per-suite outcomes alongside the full ```ts const result = await verifier.verifyCredential({ credential }); -// result.summary[i] = { id, phase, suite, status, verified, message, counts, ... } +// result.summary[i] = { id, phase, suite, status, hasFailures, message, counts, ... } // result.results[] = every check that ran, was skipped, or was not run, -// each with .id populated +// each with its .id // Detail rows for one summary entry — filter by status for just the failures. const failures = result.results.filter( - r => r.id?.startsWith(result.summary[0].id + '.') && + r => r.id.startsWith(result.summary[0].id + '.') && r.outcome.status === 'failure' ); ``` Render from `summary[]` and lazy-expand into `results[]`. +Check ids and the built-in suites' `phase` tags are fixed for 2.x. An id +embeds its suite's phase, so re-tagging a built-in suite would rename every +check in it: changing either is a breaking change. + See [`docs/api/verification-results.md`](./docs/api/verification-results.md) for the full reference (phase model, `SuiteSummary` fields, `id` namespace, UI rendering recipe, and a prompt-ready appendix for downstream UIs). diff --git a/docs/api/timing.md b/docs/api/timing.md index 85fbac1..4beb835 100644 --- a/docs/api/timing.md +++ b/docs/api/timing.md @@ -310,7 +310,7 @@ function buildBreakdown(result: PresentationVerificationResult) { bySuite: cr.summary.map(s => ({ suite: s.id, durationMs: s.timing!.durationMs, - verified: s.verified, + hasFailures: s.hasFailures, })), })), }; diff --git a/docs/api/verification-results.md b/docs/api/verification-results.md index 9b02673..970b55e 100644 --- a/docs/api/verification-results.md +++ b/docs/api/verification-results.md @@ -55,6 +55,10 @@ A successful single-credential verification: "suite": "status", "check": "status.bitstring", "outcome": { "status": "skipped", "reason": "Credential has no credentialStatus." } }, + { "id": "cryptographic.status.list-issuer", + "suite": "status", "check": "status.list-issuer", + "outcome": { "status": "skipped", + "reason": "No status list was checked." } }, { "id": "trust.registry.issuer", "suite": "registry", "check": "registry.issuer", "outcome": { @@ -74,23 +78,23 @@ A successful single-credential verification: ], "summary": [ { "id": "cryptographic.core", "phase": "cryptographic", - "suite": "core", "status": "success", "verified": true, + "suite": "core", "status": "success", "hasFailures": false, "message": "5 of 5 checks passed", "counts": { "passed": 5, "failed": 0, "skipped": 0 } }, { "id": "recognition", "phase": "recognition", - "suite": "recognition", "status": "skipped", "verified": true, + "suite": "recognition", "status": "skipped", "hasFailures": false, "message": "1 of 1 check skipped", "counts": { "passed": 0, "failed": 0, "skipped": 1 } }, { "id": "cryptographic.proof", "phase": "cryptographic", - "suite": "proof", "status": "success", "verified": true, + "suite": "proof", "status": "success", "hasFailures": false, "message": "1 of 1 check passed", "counts": { "passed": 1, "failed": 0, "skipped": 0 } }, { "id": "cryptographic.status", "phase": "cryptographic", - "suite": "status", "status": "skipped", "verified": true, - "message": "1 of 1 check skipped", - "counts": { "passed": 0, "failed": 0, "skipped": 1 } }, + "suite": "status", "status": "skipped", "hasFailures": false, + "message": "2 of 2 checks skipped", + "counts": { "passed": 0, "failed": 0, "skipped": 2 } }, { "id": "trust.registry", "phase": "trust", - "suite": "registry","status": "success", "verified": true, + "suite": "registry","status": "success", "hasFailures": false, "message": "1 of 1 check passed", "counts": { "passed": 1, "failed": 0, "skipped": 0 } } ] @@ -232,8 +236,12 @@ interface SuiteSummary { /** Suite id (matches VerificationSuite.id). */ suite: string; status: 'success' | 'failure' | 'skipped' | 'mixed'; - /** Convenience: `status !== 'failure' && status !== 'mixed'`. */ - verified: boolean; + /** + * True iff at least one check in this suite failed — + * `status === 'failure' || status === 'mixed'`. Deliberately not + * called `verified`: only the top-level `verified` is a verdict. + */ + hasFailures: boolean; message: string; counts: { passed: number; failed: number; skipped: number }; /** @@ -273,15 +281,27 @@ interface SuiteSummary { | `mixed` | ⚠ | amber | Both passes and failures in the same suite | | `skipped` | — | gray | Suite was acknowledged but produced no run checks | -`verified === false` iff `status === 'failure' || status === 'mixed'`. A +`hasFailures === true` iff `status === 'failure' || status === 'mixed'`. +It is a per-suite fact, not a verdict: a non-fatal failure sets it on a +suite inside a result whose top-level `verified` is still `true`. The top-level `result.verified` is the AND of every credential's `result.verified` AND no presentation-level fatal failures. ## `id` namespace reference Every `CheckResult` has an `id` of the form -`..`. Every `SuiteSummary` has the matching -`.` prefix (no trailing local part). Built-in ids: +`..` — the suite runner sets it on every row +it emits, so it is always there. Every `SuiteSummary` has the matching +`.` prefix (no trailing local part). + +> **Stable for 2.x.** Check ids and the built-in suites' `phase` tags +> are API. Because an id embeds its suite's phase, re-tagging a +> built-in suite renames every check in it — so changing either a +> check id or a built-in phase tag is a breaking change, not a +> refactor. + +Every id the default suites, the parse-failure path and the +`/openbadges` suites can produce: ``` cryptographic.core @@ -295,25 +315,37 @@ cryptographic.proof cryptographic.proof.signature cryptographic.status cryptographic.status.bitstring + cryptographic.status.list-issuer trust.registry trust.registry.issuer recognition recognition.profile +cryptographic.parsing (parse failures only) + cryptographic.parsing.envelope semantic.openbadges (openBadgesSuite) + semantic.openbadges.applies semantic.openbadges.schema.obv3.result-ref semantic.openbadges.schema.obv3.achieved-level semantic.openbadges.schema.obv3.missing-result-status semantic.openbadges.schema.obv3.unknown-achievement-type semantic.openbadges.schema.obv3.json semantic.openbadges.semantic (openBadgesSemanticSuite) + semantic.openbadges.semantic.applies semantic.openbadges.semantic.schema.obv3.result-ref semantic.openbadges.semantic.schema.obv3.achieved-level semantic.openbadges.semantic.schema.obv3.missing-result-status semantic.openbadges.semantic.schema.obv3.unknown-achievement-type semantic.openbadges.schema (openBadgesSchemaSuite) + semantic.openbadges.schema.applies semantic.openbadges.schema.schema.obv3.json ``` +The three `*.applies` rows appear only when the suite was queued +explicitly via `additionalSuites` and its predicate rejected the +credential, or when that predicate threw; see rule 5 below. A suite +supplied by a consumer without a `phase` tag produces ids beginning +`unknown.` (e.g. `unknown.my-suite.my-check`). + ### Computation rules `computeId(phase, suite, localCheckId)` (also exported from the @@ -394,7 +426,7 @@ function failureDetail( failing: SuiteSummary, ) { return cr.results.filter( - r => r.id?.startsWith(failing.id + '.') && r.outcome.status === 'failure', + r => r.id.startsWith(failing.id + '.') && r.outcome.status === 'failure', ); } @@ -429,9 +461,11 @@ Lazy-expand them — most users only want the verdict + per-phase status. ## Backwards compatibility -- **Deprecated.** `CheckResult.check` and `CheckResult.suite` are - marked `@deprecated`. Use `CheckResult.id` instead. Removal target: - the major after v2.0.0. +- **Deprecated.** `CheckResult.check` is marked `@deprecated`. Use + `CheckResult.id` instead. Removal target: the major after v2.0.0. + `CheckResult.suite` is **not** deprecated: it is how rows are + grouped, it matches `SuiteSummary.suite`, and it cannot be + recovered from `id`, because a suite id may itself contain dots. - **`flattenPresentationResults`** is unchanged. Use it as the "give me one array of every check" escape hatch when you don't want to walk `credentialResults[].results`. @@ -474,7 +508,7 @@ dcc-transaction-service UI implementation. ], "summary": [ { "id": "cryptographic.proof", "phase": "cryptographic", - "suite": "proof", "status": "success", "verified": true, + "suite": "proof", "status": "success", "hasFailures": false, "message": "1 of 1 check passed", "counts": { "passed": 1, "failed": 0, "skipped": 0 } } ], @@ -495,19 +529,19 @@ dcc-transaction-service UI implementation. ], "summary": [ { "id": "cryptographic.core", "phase": "cryptographic", - "suite": "core", "status": "success", "verified": true, + "suite": "core", "status": "success", "hasFailures": false, "message": "5 of 5 checks passed", "counts": { "passed": 5, "failed": 0, "skipped": 0 } }, { "id": "recognition", "phase": "recognition", - "suite": "recognition", "status": "success", "verified": true, + "suite": "recognition", "status": "success", "hasFailures": false, "message": "1 of 1 check passed", "counts": { "passed": 1, "failed": 0, "skipped": 0 } }, { "id": "cryptographic.proof", "phase": "cryptographic", - "suite": "proof", "status": "success", "verified": true, + "suite": "proof", "status": "success", "hasFailures": false, "message": "1 of 1 check passed", "counts": { "passed": 1, "failed": 0, "skipped": 0 } }, { "id": "trust.registry", "phase": "trust", - "suite": "registry", "status": "success", "verified": true, + "suite": "registry", "status": "success", "hasFailures": false, "message": "1 of 1 check passed", "counts": { "passed": 1, "failed": 0, "skipped": 0 } } ] @@ -536,19 +570,19 @@ dcc-transaction-service UI implementation. ], "summary": [ { "id": "cryptographic.core", "phase": "cryptographic", - "suite": "core", "status": "success", "verified": true, + "suite": "core", "status": "success", "hasFailures": false, "message": "5 of 5 checks passed", "counts": { "passed": 5, "failed": 0, "skipped": 0 } }, { "id": "recognition", "phase": "recognition", - "suite": "recognition", "status": "success", "verified": true, + "suite": "recognition", "status": "success", "hasFailures": false, "message": "1 of 1 check passed", "counts": { "passed": 1, "failed": 0, "skipped": 0 } }, { "id": "cryptographic.proof", "phase": "cryptographic", - "suite": "proof", "status": "success", "verified": true, + "suite": "proof", "status": "success", "hasFailures": false, "message": "1 of 1 check passed", "counts": { "passed": 1, "failed": 0, "skipped": 0 } }, { "id": "trust.registry", "phase": "trust", - "suite": "registry", "status": "failure", "verified": false, + "suite": "registry", "status": "failure", "hasFailures": true, "message": "1 of 1 check failed", "counts": { "passed": 0, "failed": 1, "skipped": 0 } } ] @@ -572,7 +606,7 @@ interface SuiteSummary { phase: SuiteSummaryPhase; suite: string; status: 'success' | 'failure' | 'skipped' | 'mixed'; - verified: boolean; + hasFailures: boolean; message: string; counts: { passed: number; failed: number; skipped: number }; fatalFailureAt?: string; @@ -602,7 +636,7 @@ function failureDetail( failing: SuiteSummary, ) { return cr.results.filter( - r => r.id?.startsWith(failing.id + '.') && r.outcome.status === 'failure', + r => r.id.startsWith(failing.id + '.') && r.outcome.status === 'failure', ); } ``` diff --git a/docs/architecture.md b/docs/architecture.md index 0138dd7..fc023e0 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -310,7 +310,7 @@ CheckOutcome └── { status: 'skipped', reason } │ ▼ -CheckResult: { suite, check, outcome, fatal?, timing? } +CheckResult: { id, suite, check, outcome, fatal?, timing? } ``` `timing` is opt-in: present on every `CheckResult`, diff --git a/src/compute-id.ts b/src/compute-id.ts new file mode 100644 index 0000000..b6290c1 --- /dev/null +++ b/src/compute-id.ts @@ -0,0 +1,51 @@ +/** + * The one place a dot-separated namespaced id is built. + * + * Lives apart from `fold-results.ts` because both `run-suites.ts` + * (which stamps `CheckResult.id`) and `fold-results.ts` (which + * builds `SuiteSummary.id`) need it, and `fold-results.ts` already + * imports from `run-suites.ts`. + * + * ## STABILITY: check ids are API + * + * A check id embeds its suite's `phase`, so re-tagging a built-in + * suite's phase renames every check in it. Check ids and the + * built-in suites' phase tags are fixed for 2.x: changing either is + * a breaking change, not a refactor. + */ + +import type { SuitePhase } from './types/check.js'; +import type { SuiteSummaryPhase } from './types/suite-summary.js'; + +/** + * Compute the dot-separated namespaced id for a single + * `CheckResult` or `SuiteSummary`. + * + * Rules: + * - Strip the `.` prefix from `localCheckId` if present. + * - When `phase === suite`, collapse to a single segment. + * - When `phase` is missing, use `'unknown'`. + * - When `localCheckId` is empty (suite-summary case), drop the + * trailing segment. + * + * @example + * computeId('cryptographic', 'core', 'core.proof-exists') + * // → 'cryptographic.core.proof-exists' + * computeId('recognition', 'recognition', 'recognition.profile') + * // → 'recognition.profile' + * computeId('cryptographic', 'core', '') + * // → 'cryptographic.core' + */ +export function computeId( + phase: SuitePhase | undefined, + suite: string, + localCheckId: string +): string { + const phaseSeg: SuiteSummaryPhase = phase ?? 'unknown'; + const localPart = localCheckId.startsWith(`${suite}.`) + ? localCheckId.slice(suite.length + 1) + : localCheckId; + const segments: string[] = + phaseSeg === suite ? [phaseSeg, localPart] : [phaseSeg, suite, localPart]; + return segments.filter(Boolean).join('.'); +} diff --git a/src/fold-results.ts b/src/fold-results.ts index 071ca44..3374e92 100644 --- a/src/fold-results.ts +++ b/src/fold-results.ts @@ -27,9 +27,12 @@ import type { SuitePhase, VerificationSuite } from './types/check.js'; -import type { SuiteSummary, SuiteSummaryPhase } from './types/suite-summary.js'; +import type { SuiteSummary } from './types/suite-summary.js'; import type { TaskTiming } from './types/timing.js'; import { NOT_RUN_REASON_PREFIX } from './run-suites.js'; +import { computeId } from './compute-id.js'; + +export { computeId }; export function foldCheckResults( checks: CheckResult[], @@ -52,39 +55,6 @@ export function foldCheckResults( return { results: checks, summaries }; } -/** - * Compute the dot-separated namespaced id for a single - * `CheckResult` or `SuiteSummary`. - * - * Rules: - * - Strip the `.` prefix from `localCheckId` if present. - * - When `phase === suite`, collapse to a single segment. - * - When `phase` is missing, use `'unknown'`. - * - When `localCheckId` is empty (suite-summary case), drop the - * trailing segment. - * - * @example - * computeId('cryptographic', 'core', 'core.proof-exists') - * // → 'cryptographic.core.proof-exists' - * computeId('recognition', 'recognition', 'recognition.profile') - * // → 'recognition.profile' - * computeId('cryptographic', 'core', '') - * // → 'cryptographic.core' - */ -export function computeId( - phase: SuitePhase | undefined, - suite: string, - localCheckId: string -): string { - const phaseSeg: SuiteSummaryPhase = phase ?? 'unknown'; - const localPart = localCheckId.startsWith(`${suite}.`) - ? localCheckId.slice(suite.length + 1) - : localCheckId; - const segments: string[] = - phaseSeg === suite ? [phaseSeg, localPart] : [phaseSeg, suite, localPart]; - return segments.filter(Boolean).join('.'); -} - // ---------------------------------------------------------------- // helpers // ---------------------------------------------------------------- @@ -140,12 +110,10 @@ function summarizeSuite( const fatalCheck = suiteChecks.find( c => c.fatal === true && c.outcome.status === 'failure' ); - const fatalFailureAt = fatalCheck - ? (fatalCheck.id ?? computeId(phase, suiteId, fatalCheck.check)) - : undefined; + const fatalFailureAt = fatalCheck?.id; const status = deriveStatus(counts); - const verified = counts.failed === 0; + const hasFailures = counts.failed > 0; const message = formatMessage( suiteId, @@ -162,7 +130,7 @@ function summarizeSuite( phase: phase ?? 'unknown', suite: suiteId, status, - verified, + hasFailures, message, counts, ...(fatalFailureAt !== undefined ? { fatalFailureAt } : {}), diff --git a/src/run-suites.ts b/src/run-suites.ts index e066076..b974945 100644 --- a/src/run-suites.ts +++ b/src/run-suites.ts @@ -14,6 +14,9 @@ * - A throw never escapes. A throwing `execute` or `applies` becomes a * `failure` result carrying {@link ProblemTypes.CHECK_ERROR}, so * `runSuites` always resolves. + * - Every row carries its namespaced `id` (see {@link computeId}), + * including not-run rows and both kinds of synthetic `.applies` + * row. Nothing downstream has to fill one in. * * This module is pure orchestration — it has no knowledge of what any * check actually does. @@ -28,6 +31,7 @@ import { } from './types/check.js'; import { VerificationContext } from './types/context.js'; import { ProblemTypes } from './problem-types.js'; +import { computeId } from './compute-id.js'; import { VerificationSubject } from './types/subject.js'; import type { TimeService } from './services/time-service/time-service.js'; import type { TaskTiming } from './types/timing.js'; @@ -150,7 +154,7 @@ export async function runSuites( } catch (error) { const fatal = suite.checks.some(c => c.fatal === true); results.push( - buildAppliesErrorResult(suite.id, error, fatal, timing, timeService) + buildAppliesErrorResult(suite, error, fatal, timing, timeService) ); if (fatal && haltedBy === undefined) { haltedBy = `${suite.id}.applies`; @@ -162,7 +166,7 @@ export async function runSuites( if (!applies) { if (options.explicitSuiteIds?.has(suite.id)) { results.push( - buildSyntheticAppliesSkipResult(suite.id, timing, timeService) + buildSyntheticAppliesSkipResult(suite, timing, timeService) ); } continue; @@ -175,7 +179,7 @@ export async function runSuites( if (haltedBy !== undefined) { results.push( - buildNotRunResult(suite.id, check, haltedBy, timing, timeService) + buildNotRunResult(suite, check, haltedBy, timing, timeService) ); continue; } @@ -192,6 +196,7 @@ export async function runSuites( : undefined; const result: CheckResult = { + id: computeId(suite.phase, suite.id, check.id), check: check.id, suite: suite.id, outcome, @@ -236,16 +241,17 @@ function buildCheckErrorOutcome(id: string, error: unknown): CheckOutcome { * decision, like {@link buildSyntheticAppliesSkipResult}. */ function buildAppliesErrorResult( - suiteId: string, + suite: VerificationSuite, error: unknown, fatal: boolean, timing: boolean, timeService: TimeService | undefined ): CheckResult { - const checkId = `${suiteId}.applies`; + const checkId = `${suite.id}.applies`; const result: CheckResult = { + id: computeId(suite.phase, suite.id, checkId), check: checkId, - suite: suiteId, + suite: suite.id, outcome: buildCheckErrorOutcome(checkId, error), fatal }; @@ -259,15 +265,16 @@ function buildAppliesErrorResult( * {@link buildSyntheticAppliesSkipResult}. */ function buildNotRunResult( - suiteId: string, + suite: VerificationSuite, check: VerificationCheck, haltedBy: string, timing: boolean, timeService: TimeService | undefined ): CheckResult { const result: CheckResult = { + id: computeId(suite.phase, suite.id, check.id), check: check.id, - suite: suiteId, + suite: suite.id, outcome: { status: 'skipped', reason: `${NOT_RUN_REASON_PREFIX}${haltedBy} failed` @@ -287,13 +294,15 @@ function buildNotRunResult( * the predicate itself. */ function buildSyntheticAppliesSkipResult( - suiteId: string, + suite: VerificationSuite, timing: boolean, timeService: TimeService | undefined ): CheckResult { + const checkId = `${suite.id}.applies`; const result: CheckResult = { - check: `${suiteId}.applies`, - suite: suiteId, + id: computeId(suite.phase, suite.id, checkId), + check: checkId, + suite: suite.id, outcome: { status: 'skipped', reason: 'suite predicate returned false' diff --git a/src/types/check.ts b/src/types/check.ts index 433a86e..e5bc516 100644 --- a/src/types/check.ts +++ b/src/types/check.ts @@ -50,14 +50,16 @@ export interface CheckResult { * Dot-separated namespaced id — `..` * (e.g. `"cryptographic.proof.signature"`). When `phase === suite` * the duplicate segment collapses (e.g. `"recognition.profile"`). + * A suite with no `phase` produces ids beginning `"unknown."`. * - * Computed by the verifier post-`runSuites`. Prefer this over the - * legacy `check` / `suite` pair when consuming results. + * Set by `runSuites` on every row it emits. This is the identity + * to match on; prefer it over the legacy `check` field. * - * Optional today for backwards compatibility with hand-constructed - * `CheckResult` literals; will become required in a future major. + * Check ids are API: they are fixed for 2.x, and because an id + * embeds its suite's `phase`, re-tagging a built-in suite's phase + * renames its checks and is a breaking change. */ - id?: string; + id: string; /** * @deprecated Use {@link CheckResult.id}. Will be removed in the @@ -68,10 +70,10 @@ export interface CheckResult { check: string; /** - * @deprecated Use {@link CheckResult.id}. Will be removed in the - * major release after the one that introduced `id`. - * - * Suite id this check belongs to, e.g. `"core"`. + * Suite id this check belongs to, e.g. `"core"`; mirrors + * {@link SuiteSummary.suite} and is how rows are grouped into + * summaries. Not recoverable from {@link CheckResult.id}, because + * suite names may themselves contain dots. */ suite: string; diff --git a/src/types/suite-summary.ts b/src/types/suite-summary.ts index 304c2f1..1adf568 100644 --- a/src/types/suite-summary.ts +++ b/src/types/suite-summary.ts @@ -64,8 +64,17 @@ export interface SuiteSummary { */ status: 'success' | 'failure' | 'skipped' | 'mixed'; - /** True iff no child check returned `'failure'`. */ - verified: boolean; + /** + * True iff at least one child check returned `'failure'`, + * whether or not that check was fatal. + * + * Deliberately not called `verified`: the only `verified` in a + * result is the top-level one, which means "no fatal check + * failed" across the whole subject. A suite with a non-fatal + * failure has `hasFailures: true` inside a `verified: true` + * result, and that is not a contradiction. + */ + hasFailures: boolean; /** * Human-readable rollup message. Conventions: diff --git a/src/verifier.ts b/src/verifier.ts index ac83896..532b900 100644 --- a/src/verifier.ts +++ b/src/verifier.ts @@ -82,7 +82,8 @@ import { extractCredentialsFrom } from './extract-credentials-from.js'; import { defaultSuites } from './default-suites.js'; import { coreSuite } from './suites/core/index.js'; import { proofSuite } from './suites/proof/index.js'; -import { computeId, foldCheckResults } from './fold-results.js'; +import { foldCheckResults } from './fold-results.js'; +import { computeId } from './compute-id.js'; import { defaultHttpGetService, createDefaultCacheService, @@ -176,8 +177,6 @@ export function createVerifier(config: VerifierConfig = {}): Verifier { const recognized = extractRecognition(rawChecks); - populateCheckIds(rawChecks, suites); - const folded = foldCheckResults(rawChecks, suites); const result: CredentialVerificationResult = { @@ -246,8 +245,6 @@ export function createVerifier(config: VerifierConfig = {}): Verifier { { explicitSuiteIds, phases: effectivePhases } ); - populateCheckIds(rawPresentationChecks, presentationSuites); - const foldedPresentation = foldCheckResults( rawPresentationChecks, presentationSuites @@ -338,25 +335,6 @@ function hasFatalFailures(results: CheckResult[]): boolean { return results.some(r => r.fatal && r.outcome.status === 'failure'); } -/** - * Populate `id` on every `CheckResult` using {@link computeId} so - * `results[]` and `summary[]` both carry the dot-separated - * namespace. Mutates each result in place; called after - * `runSuites` returns and before {@link foldCheckResults}. - */ -function populateCheckIds( - checks: CheckResult[], - suites: VerificationSuite[] -): void { - const phaseBySuiteId = new Map(); - for (const s of suites) { - phaseBySuiteId.set(s.id, s.phase); - } - for (const c of checks) { - c.id = computeId(phaseBySuiteId.get(c.suite), c.suite, c.check); - } -} - /** * Apply the auto-include rule for phase requests: if `'semantic'` * is requested without `'recognition'`, add `'recognition'` so @@ -428,12 +406,12 @@ function parseErrorResult( timeService: TimeService | undefined ): CheckResult { const result: CheckResult = { + id: computeId(PARSING_SUITE.phase, 'parsing', 'parsing.envelope'), suite: 'parsing', check: 'parsing.envelope', outcome: { status: 'failure', problems: [problem] }, fatal: true }; - result.id = computeId(PARSING_SUITE.phase, 'parsing', 'parsing.envelope'); if (timeService !== undefined) { // Parse failures short-circuit before any check runs, so the // synthetic `parsing.envelope` check is the only thing the @@ -458,7 +436,7 @@ function parseErrorResult( * Build the one-entry `summary[]` that accompanies every * parse-error result. The single check is always a fatal failure, * so the summary always has `status: 'failure'`, - * `verified: false`. + * `hasFailures: true`. * * `foldCheckResults` rolls per-check timing into the summary, so * the suite-level `timing` propagates here too when the call ran diff --git a/test/flatten-presentation-results.spec.ts b/test/flatten-presentation-results.spec.ts index 4ecc19d..8e8111f 100644 --- a/test/flatten-presentation-results.spec.ts +++ b/test/flatten-presentation-results.spec.ts @@ -10,6 +10,7 @@ import type { VerifiablePresentation } from '../src/schemas/presentation.js'; function mkCheck(suite: string, checkId: string): CheckResult { return { + id: `cryptographic.${suite}.${checkId}`, suite, check: checkId, outcome: { status: 'success', message: 'ok' } diff --git a/test/fold-results.spec.ts b/test/fold-results.spec.ts index c18fd6d..976a65c 100644 --- a/test/fold-results.spec.ts +++ b/test/fold-results.spec.ts @@ -50,14 +50,33 @@ function mkSuite( return { id, name: id, checks, phase: options.phase }; } +/** + * Phase each fixture suite in this file is tagged with, so `mkResult` + * can stamp the same `id` the suite runner would have. `custom` is + * deliberately absent: it stands for an untagged suite. + */ +const FIXTURE_PHASES: Record = { + core: 'cryptographic', + proof: 'cryptographic', + status: 'cryptographic', + alpha: 'cryptographic', + registry: 'trust', + recognition: 'recognition', + extra: 'semantic', + mix: 'semantic', + beta: 'semantic' +}; + function mkResult( suite: string, check: string, status: 'success' | 'failure' | 'skipped', options: { fatal?: boolean; reason?: string } = {} ): CheckResult { + const id = computeId(FIXTURE_PHASES[suite], suite, check); if (status === 'success') { return { + id, suite, check, outcome: { status: 'success', message: 'ok' }, @@ -66,6 +85,7 @@ function mkResult( } if (status === 'failure') { return { + id, suite, check, outcome: { status: 'failure', problems: [stubProblem] }, @@ -73,6 +93,7 @@ function mkResult( }; } return { + id, suite, check, outcome: { status: 'skipped', reason: options.reason ?? 'n/a' } @@ -115,7 +136,7 @@ describe('foldCheckResults', () => { ]); expect(summaries).toHaveLength(1); expect(summaries[0].status).toBe('success'); - expect(summaries[0].verified).toBe(true); + expect(summaries[0].hasFailures).toBe(false); expect(summaries[0].counts).toEqual({ passed: 4, failed: 0, @@ -141,7 +162,7 @@ describe('foldCheckResults', () => { expect(results).toHaveLength(2); expect(summaries[0].status).toBe('failure'); - expect(summaries[0].verified).toBe(false); + expect(summaries[0].hasFailures).toBe(true); expect(summaries[0].counts).toEqual({ passed: 0, failed: 2, @@ -174,7 +195,7 @@ describe('foldCheckResults', () => { expect(results.filter(r => r.outcome.status === 'failure')).toHaveLength(1); expect(results[1].check).toBe('core.b'); expect(summaries[0].status).toBe('mixed'); - expect(summaries[0].verified).toBe(false); + expect(summaries[0].hasFailures).toBe(true); expect(summaries[0].counts).toEqual({ passed: 3, failed: 1, @@ -198,7 +219,7 @@ describe('foldCheckResults', () => { expect(results).toHaveLength(1); expect(results[0].check).toBe('extra.applies'); expect(summaries[0].status).toBe('skipped'); - expect(summaries[0].verified).toBe(true); + expect(summaries[0].hasFailures).toBe(false); expect(summaries[0].counts).toEqual({ passed: 0, failed: 0, @@ -236,7 +257,7 @@ describe('foldCheckResults', () => { 'proof.d' ]); expect(summaries[0].status).toBe('mixed'); - expect(summaries[0].verified).toBe(false); + expect(summaries[0].hasFailures).toBe(true); expect(summaries[0].counts).toEqual({ passed: 1, failed: 1, @@ -296,7 +317,7 @@ describe('foldCheckResults', () => { 'proof not run: core.context-exists failed' ); expect(summaries[1].status).toBe('skipped'); - expect(summaries[1].verified).toBe(true); + expect(summaries[1].hasFailures).toBe(false); expect(summaries[1].counts).toEqual({ passed: 0, failed: 0, diff --git a/test/run-suites.spec.ts b/test/run-suites.spec.ts index 7204e0c..41e5475 100644 --- a/test/run-suites.spec.ts +++ b/test/run-suites.spec.ts @@ -84,6 +84,88 @@ describe('runSuites', () => { }); }); + describe('namespaced ids', () => { + it('stamps `..` on every row it writes', async () => { + const suite: VerificationSuite = { + id: 'core', + name: 'Core', + phase: 'cryptographic', + checks: [ + createMockCheck('core.a', 'failure', { fatal: true }), + createMockCheck('core.b', 'success') + ] + }; + + const results = await runSuites( + [suite], + { verifiableCredential: {} }, + mockContext + ); + + // The second row is the not-run row the fatal failure produced. + expect(results.map(r => r.id)).toEqual([ + 'cryptographic.core.a', + 'cryptographic.core.b' + ]); + }); + + it('uses "unknown" as the phase segment for an untagged suite', async () => { + const suite: VerificationSuite = { + id: 'custom', + name: 'Custom', + checks: [createMockCheck('custom.x', 'success')] + }; + + const results = await runSuites( + [suite], + { verifiableCredential: {} }, + mockContext + ); + + expect(results.map(r => r.id)).toEqual(['unknown.custom.x']); + }); + + it('stamps the synthetic `.applies` skip of an explicitly queued suite', async () => { + const suite: VerificationSuite = { + id: 'openbadges', + name: 'Open Badges', + phase: 'semantic', + applies: () => false, + checks: [createMockCheck('openbadges.x', 'success')] + }; + + const results = await runSuites( + [suite], + { verifiableCredential: {} }, + mockContext, + { explicitSuiteIds: new Set(['openbadges']) } + ); + + expect(results.map(r => r.id)).toEqual(['semantic.openbadges.applies']); + }); + + it('stamps the `.applies` failure of a throwing predicate', async () => { + const suite: VerificationSuite = { + id: 'openbadges', + name: 'Open Badges', + phase: 'semantic', + applies: () => { + throw new Error('boom'); + }, + checks: [createMockCheck('openbadges.x', 'success')] + }; + + const results = await runSuites( + [suite], + { verifiableCredential: {} }, + mockContext + ); + + expect(results.map(r => r.id)).toEqual(['semantic.openbadges.applies']); + expect(results[0].outcome.status).toBe('failure'); + }); + }); + // A check that records whether it executed. const spyCheck = ( id: string, diff --git a/test/types.spec.ts b/test/types.spec.ts index 507c6eb..47b27f3 100644 --- a/test/types.spec.ts +++ b/test/types.spec.ts @@ -82,6 +82,7 @@ describe('Foundation types', () => { describe('CheckResult', () => { it('tags outcome with suite and check ids', () => { const result: CheckResult = { + id: 'cryptographic.core.proof-exists', check: 'core.proof-exists', suite: 'core', outcome: { status: 'success', message: 'Proof exists.' } @@ -91,7 +92,7 @@ describe('Foundation types', () => { expect(result.outcome.status).toBe('success'); }); - it('accepts an optional namespaced id', () => { + it('requires a namespaced id', () => { const result: CheckResult = { id: 'cryptographic.core.proof-exists', check: 'core.proof-exists', @@ -109,7 +110,7 @@ describe('Foundation types', () => { phase: 'cryptographic', suite: 'core', status: 'success', - verified: true, + hasFailures: false, message: '4 of 4 checks passed', counts: { passed: 4, failed: 0, skipped: 0 } }; @@ -123,7 +124,7 @@ describe('Foundation types', () => { phase: 'cryptographic', suite: 'proof', status: 'failure', - verified: false, + hasFailures: true, message: '1 of 3 checks failed (0 passed)', counts: { passed: 0, failed: 1, skipped: 0 }, fatalFailureAt: 'proof.signature' @@ -138,7 +139,7 @@ describe('Foundation types', () => { phase, suite: 'custom', status: 'success', - verified: true, + hasFailures: false, message: '1 of 1 checks passed', counts: { passed: 1, failed: 0, skipped: 0 } }; @@ -154,7 +155,7 @@ describe('Foundation types', () => { phase: 'recognition', suite: 'recognition', status: 'skipped', - verified: true, + hasFailures: false, message: 'no recognizer matched', counts: { passed: 0, failed: 0, skipped: 1 } }; diff --git a/test/verify-credential.spec.ts b/test/verify-credential.spec.ts index 4a1bcd9..06dba30 100644 --- a/test/verify-credential.spec.ts +++ b/test/verify-credential.spec.ts @@ -582,7 +582,7 @@ describe('verifyCredential', () => { }); expect(result.summary.length).toBeGreaterThan(0); - expect(result.summary.every(s => s.verified)).toBe(true); + expect(result.summary.every(s => !s.hasFailures)).toBe(true); const ids = new Set(result.summary.map(s => s.id)); expect(ids).toContain('cryptographic.core'); expect(ids).toContain('cryptographic.proof'); @@ -617,7 +617,7 @@ describe('verifyCredential', () => { ).toBe(true); const proofSummary = result.summary.find(s => s.suite === 'proof'); expect(['failure', 'mixed']).toContain(proofSummary?.status); - expect(proofSummary?.verified).toBe(false); + expect(proofSummary?.hasFailures).toBe(true); }); it('parse error: result has one summary entry tagged cryptographic.parsing', async () => { diff --git a/test/verify-presentation.spec.ts b/test/verify-presentation.spec.ts index 0a2feb0..a60b035 100644 --- a/test/verify-presentation.spec.ts +++ b/test/verify-presentation.spec.ts @@ -463,14 +463,14 @@ describe('verifyPresentation', () => { true ); expect(result.summary.length).toBeGreaterThan(0); - expect(result.summary.every(s => s.verified)).toBe(true); + expect(result.summary.every(s => !s.hasFailures)).toBe(true); expect(result.credentialResults).toHaveLength(1); const cred = result.credentialResults[0]; expect(cred.results.length).toBeGreaterThan(0); expect(cred.results.every(r => r.id !== undefined)).toBe(true); expect(cred.summary.length).toBeGreaterThan(0); - expect(cred.summary.every(s => s.verified)).toBe(true); + expect(cred.summary.every(s => !s.hasFailures)).toBe(true); }); }); @@ -502,7 +502,7 @@ describe('verifyPresentation', () => { const result = await verifyPresentation({ presentation, ...cryptoOnly }); expect(result.summary.length).toBeGreaterThan(0); - expect(result.summary.every(s => s.verified)).toBe(true); + expect(result.summary.every(s => !s.hasFailures)).toBe(true); }); it('passing credential summary is all green; failing credential surfaces a failure entry', async () => { @@ -514,13 +514,13 @@ describe('verifyPresentation', () => { expect(passing).toBeDefined(); expect(failing).toBeDefined(); - expect(passing!.summary.every(s => s.verified)).toBe(true); + expect(passing!.summary.every(s => !s.hasFailures)).toBe(true); expect(passing!.results.length).toBeGreaterThan(0); expect( passing!.results.every(r => r.outcome.status !== 'failure') ).toBe(true); - const failureSummaries = failing!.summary.filter(s => !s.verified); + const failureSummaries = failing!.summary.filter(s => s.hasFailures); expect(failureSummaries.length).toBeGreaterThan(0); expect( failing!.results.filter(r => r.outcome.status === 'failure').length @@ -532,12 +532,12 @@ describe('verifyPresentation', () => { const result = await verifyPresentation({ presentation, ...cryptoOnly }); const failing = result.credentialResults.find(c => !c.verified)!; - const failingSummary = failing.summary.find(s => !s.verified)!; + const failingSummary = failing.summary.find(s => s.hasFailures)!; // `results[]` carries passes and skips too, so a prefix match // has to be narrowed by status to reach the failure detail. const detail = failing.results.filter( r => - r.id?.startsWith(failingSummary.id + '.') && + r.id.startsWith(failingSummary.id + '.') && r.outcome.status === 'failure' ); expect(detail.length).toBeGreaterThan(0); From 38b1de0a3e9c35d14657de790e38d8535d419a9e Mon Sep 17 00:00:00 2001 From: Nate Otto Date: Tue, 20 Oct 2026 13:57:54 -0500 Subject: [PATCH 2/2] feat(results): remove SuiteSummary.hasFailures; document fatalFailureAt `hasFailures` was always `status === 'failure' || status === 'mixed'` and nothing read it, so it goes before the 2.0 shape freeze. Adding a field later is not a breaking change; removing one is. `fatalFailureAt` is now documented as the per-suite fatal signal. A fatal failure halts the subject, so at most one summary carries it, and a result is unverified exactly when one of its summaries does. `status` stays a plain count-based fact; the docs now tell UIs to colour the suite with `fatalFailureAt` as an error and other failing suites as warnings, so a non-fatal failure no longer reads as red inside a verified credential. Docs fixes in the same pass: - `verified` on both result types said "no check failed"; it means no fatal check failed. - The API guide's rendering example and prompt appendix showed a credential as unverified because of a registry failure, which is non-fatal. The examples now show a verified credential with warnings and an unverified one with a fatal signature failure. - `'mixed'` was documented as covering failure plus skipped; the code returns `'failure'` for that case. Tests cover a non-fatal custom failure (suite fails, no `fatalFailureAt`, still verified) and the one-fatal-summary invariant. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 13 ++- README.md | 2 +- docs/api/timing.md | 2 +- docs/api/verification-results.md | 182 ++++++++++++++++++++----------- src/fold-results.ts | 2 - src/types/result.ts | 10 +- src/types/suite-summary.ts | 36 +++--- src/verifier.ts | 4 +- test/fold-results.spec.ts | 9 +- test/types.spec.ts | 6 +- test/verify-credential.spec.ts | 43 +++++++- test/verify-presentation.spec.ts | 21 +++- 12 files changed, 218 insertions(+), 112 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78548ba..ef9fd85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,11 +14,14 @@ not run after a fatal failure. `@interop/ecdsa-multikey`. `defaultCryptoSuites()` now registers three suites: Ed25519Signature2020, EdDSA RDFC-2022, and ECDSA RDFC-2019. - `SuiteSummary` type and `summary: SuiteSummary[]` field on - `CredentialVerificationResult` and `PresentationVerificationResult`. Each - entry carries `hasFailures: boolean` — true when any check in that suite - failed. It is not called `verified`: only the top-level `verified` is a - verdict ("no fatal check failed"), and a non-fatal failure sets - `hasFailures` on a suite inside a result that is still `verified: true`. + `CredentialVerificationResult` and `PresentationVerificationResult`. A + summary has no `verified` field: the only verdict is the result-level + `verified` ("no fatal check failed"). `status` comes from the suite's + counts, so a non-fatal failure shows as `status: 'failure'` or `'mixed'` + inside a result that is still `verified: true`. The one suite whose check + failed fatally carries `fatalFailureAt`, and a result is unverified exactly + when one of its summaries has it; UIs should colour that suite as an error + and other failing suites as warnings. - Required `id: string` field on `CheckResult` — dot-separated `..` namespace, set by the suite runner on every row it emits (including not-run rows and synthetic `.applies` diff --git a/README.md b/README.md index c9c38e6..25cf508 100644 --- a/README.md +++ b/README.md @@ -811,7 +811,7 @@ a `summary: SuiteSummary[]` rollup of per-suite outcomes alongside the full ```ts const result = await verifier.verifyCredential({ credential }); -// result.summary[i] = { id, phase, suite, status, hasFailures, message, counts, ... } +// result.summary[i] = { id, phase, suite, status, message, counts, fatalFailureAt?, ... } // result.results[] = every check that ran, was skipped, or was not run, // each with its .id diff --git a/docs/api/timing.md b/docs/api/timing.md index 4beb835..8f49d44 100644 --- a/docs/api/timing.md +++ b/docs/api/timing.md @@ -310,7 +310,7 @@ function buildBreakdown(result: PresentationVerificationResult) { bySuite: cr.summary.map(s => ({ suite: s.id, durationMs: s.timing!.durationMs, - hasFailures: s.hasFailures, + fatalFailureAt: s.fatalFailureAt, })), })), }; diff --git a/docs/api/verification-results.md b/docs/api/verification-results.md index 970b55e..1c69f06 100644 --- a/docs/api/verification-results.md +++ b/docs/api/verification-results.md @@ -78,23 +78,23 @@ A successful single-credential verification: ], "summary": [ { "id": "cryptographic.core", "phase": "cryptographic", - "suite": "core", "status": "success", "hasFailures": false, + "suite": "core", "status": "success", "message": "5 of 5 checks passed", "counts": { "passed": 5, "failed": 0, "skipped": 0 } }, { "id": "recognition", "phase": "recognition", - "suite": "recognition", "status": "skipped", "hasFailures": false, + "suite": "recognition", "status": "skipped", "message": "1 of 1 check skipped", "counts": { "passed": 0, "failed": 0, "skipped": 1 } }, { "id": "cryptographic.proof", "phase": "cryptographic", - "suite": "proof", "status": "success", "hasFailures": false, + "suite": "proof", "status": "success", "message": "1 of 1 check passed", "counts": { "passed": 1, "failed": 0, "skipped": 0 } }, { "id": "cryptographic.status", "phase": "cryptographic", - "suite": "status", "status": "skipped", "hasFailures": false, + "suite": "status", "status": "skipped", "message": "2 of 2 checks skipped", "counts": { "passed": 0, "failed": 0, "skipped": 2 } }, { "id": "trust.registry", "phase": "trust", - "suite": "registry","status": "success", "hasFailures": false, + "suite": "registry","status": "success", "message": "1 of 1 check passed", "counts": { "passed": 1, "failed": 0, "skipped": 0 } } ] @@ -235,19 +235,14 @@ interface SuiteSummary { phase: SuiteSummaryPhase; // SuitePhase | 'unknown' /** Suite id (matches VerificationSuite.id). */ suite: string; + /** Computed from counts alone; says nothing about whether a failure was fatal. */ status: 'success' | 'failure' | 'skipped' | 'mixed'; - /** - * True iff at least one check in this suite failed — - * `status === 'failure' || status === 'mixed'`. Deliberately not - * called `verified`: only the top-level `verified` is a verdict. - */ - hasFailures: boolean; message: string; counts: { passed: number; failed: number; skipped: number }; /** - * The fully-qualified id of the fatal check that short-circuited - * the suite, if any. Absent when the suite ran to completion or - * skipped without entering a fatal state. + * The fully-qualified id of the check in this suite that failed + * fatally, if one did. At most one suite per subject carries it, + * and that suite is why the subject is not verified. */ fatalFailureAt?: string; } @@ -272,20 +267,47 @@ interface SuiteSummary { > Singular `check` is used when `n === 1`. -### Per-status badge recommendations +### `status` values + +| `status` | Meaning | +|----------|---------| +| `success` | Every check that ran in this suite passed | +| `failure` | At least one check failed and none passed (skips allowed) | +| `mixed` | At least one check failed and at least one passed | +| `skipped` | No check in this suite ran | + +`status` is computed from `counts` alone. It does not say whether a +failure was fatal: a suite whose only failure is a non-fatal check +(for example `trust.registry.issuer`) has `status: 'failure'` inside a +result whose `verified` is still `true`. + +### Fatal failures and `verified` + +A fatal failure halts every later check for the subject, so a subject +has at most one, and at most one of its summaries carries +`fatalFailureAt`. That gives one invariant to rely on: -| `status` | Glyph | Color | Meaning | -|----------|-------|-------|---------| -| `success` | ✓ | green | Every check that ran in this suite passed | -| `failure` | ✗ | red | Every check in this suite failed (or only failures + skips) | -| `mixed` | ⚠ | amber | Both passes and failures in the same suite | -| `skipped` | — | gray | Suite was acknowledged but produced no run checks | +> A credential's `verified` is `false` exactly when one of its +> `summary[]` entries has `fatalFailureAt`. -`hasFailures === true` iff `status === 'failure' || status === 'mixed'`. -It is a per-suite fact, not a verdict: a non-fatal failure sets it on a -suite inside a result whose top-level `verified` is still `true`. The -top-level `result.verified` is the AND of every credential's -`result.verified` AND no presentation-level fatal failures. +The same holds for a presentation's own checks and its `summary[]`; the +presentation's `verified` additionally requires every +`credentialResults[i].verified`. + +There is deliberately no per-suite `verified`: the only verdicts are +the result-level `verified` fields. + +### Per-suite badge recommendations + +Colour a suite by `fatalFailureAt` first and `status` second, so a +non-fatal failure never reads as red inside a verified credential: + +| Condition | Glyph | Color | Meaning | +|-----------|-------|-------|---------| +| `fatalFailureAt` is set | ✗ | red | This suite's failure is why the subject is not verified | +| `status` is `failure` or `mixed`, no `fatalFailureAt` | ⚠ | amber | Warning: a non-fatal check failed; `verified` is unaffected | +| `status === 'success'` | ✓ | green | Every check that ran passed | +| `status === 'skipped'` | — | gray | Nothing ran (not applicable, or halted by an earlier fatal failure) | ## `id` namespace reference @@ -434,28 +456,46 @@ function failureDetail( function profileLabel(cr: CredentialVerificationResult) { return cr.recognizedProfile; // e.g. 'obv3p0.openbadge', or undefined } + +// 5. Badge for one suite (or the worst of a phase's suites). +// Check `fatalFailureAt` before `status`: a non-fatal failure is a +// warning, not an error. +type Badge = 'error' | 'warning' | 'ok' | 'none'; +function suiteBadge(s: SuiteSummary): Badge { + if (s.fatalFailureAt !== undefined) return 'error'; + if (s.status === 'failure' || s.status === 'mixed') return 'warning'; + return s.status === 'success' ? 'ok' : 'none'; +} ``` -A presentation with two credentials where one passes and one fails -trust gets rendered as: +A presentation with two credentials, one verified with warnings and +one not verified, gets rendered as: ``` Presentation: ✗ (1 of 2 credentials verified) ├─ VP signature: ✓ ├─ Credential 1 (obv3p0.openbadge): ✓ -│ ├─ cryptographic ✓ (4 + 1 + 1 = 6 checks passed) -│ ├─ trust ✓ +│ ├─ cryptographic ✓ +│ ├─ trust ⚠ (1 of 1 check failed) +│ │ └─ trust.registry.issuer — Issuer Not Registered │ ├─ recognition ✓ -│ └─ semantic ✓ (5 of 5 checks passed) [openBadgesSuite] +│ └─ semantic ⚠ (1 of 5 checks failed, 4 passed) [openBadgesSuite] +│ └─ semantic.openbadges.schema.obv3.result-ref — Invalid Result Reference └─ Credential 2 (obv3p0.openbadge): ✗ - ├─ cryptographic ✓ - ├─ trust ✗ (1 of 1 check failed) - │ └─ trust.registry.issuer — Issuer Not Registered + ├─ cryptographic ✗ (proof: 1 of 1 check failed; status: not run) + │ └─ cryptographic.proof.signature — Credential Expired + ├─ trust — (registry not run: proof.signature failed) ├─ recognition ✓ - └─ semantic ⚠ (1 of 5 checks failed, 4 passed) - └─ semantic.openbadges.schema.obv3.result-ref — Invalid Result Reference + └─ semantic — (not run: proof.signature failed) ``` +Credential 1 is verified: its registry and Open Badges checks are +non-fatal, so their failures are warnings (amber), not errors. +Credential 2 is not verified because `proof.signature` failed fatally; +its proof suite carries `fatalFailureAt: "cryptographic.proof.signature"`, +and every later check was not run. A phase takes the most severe badge +of its suites. + The `└─` rows under each failing phase row come from `failureDetail()`. Lazy-expand them — most users only want the verdict + per-phase status. @@ -485,7 +525,9 @@ dcc-transaction-service UI implementation. > (`cryptographic` → `trust` → `recognition` → `semantic` → `unknown`). > Show failure detail rows (from `results[]` filtered by `id` prefix > **and** `outcome.status === 'failure'`) under the failing phase row, -> lazy-expanded. +> lazy-expanded. Colour a suite red only when it has `fatalFailureAt`; +> a suite with failures but no `fatalFailureAt` is a warning (amber), +> because non-fatal failures leave `verified` true. > > The result is the return value of > `verifier.verifyPresentation({ presentation })` from @@ -508,14 +550,14 @@ dcc-transaction-service UI implementation. ], "summary": [ { "id": "cryptographic.proof", "phase": "cryptographic", - "suite": "proof", "status": "success", "hasFailures": false, + "suite": "proof", "status": "success", "message": "1 of 1 check passed", "counts": { "passed": 1, "failed": 0, "skipped": 0 } } ], "credentialResults": [ { "verified": true, - "verifiableCredential": { /* good VC */ }, + "verifiableCredential": { /* VC from an unregistered issuer */ }, "recognizedProfile": "obv3p0.openbadge", "results": [ { "id": "cryptographic.core.context-exists", /* success */ }, @@ -525,30 +567,40 @@ dcc-transaction-service UI implementation. { "id": "cryptographic.core.proof-exists", /* success */ }, { "id": "recognition.profile", /* success */ }, { "id": "cryptographic.proof.signature", /* success */ }, - { "id": "trust.registry.issuer", /* success */ } + { "id": "trust.registry.issuer", + "suite": "registry", "check": "registry.issuer", + "outcome": { "status": "failure", "problems": [ + { "type": "https://www.w3.org/TR/vc-data-model#ISSUER_NOT_REGISTERED", + "title": "Issuer Not Registered", + "detail": "Issuer DID not in any registry." } + ], + "payload": { "matches": [], "uncheckedRegistries": [] }}, + "fatal": false } ], "summary": [ { "id": "cryptographic.core", "phase": "cryptographic", - "suite": "core", "status": "success", "hasFailures": false, + "suite": "core", "status": "success", "message": "5 of 5 checks passed", "counts": { "passed": 5, "failed": 0, "skipped": 0 } }, { "id": "recognition", "phase": "recognition", - "suite": "recognition", "status": "success", "hasFailures": false, + "suite": "recognition", "status": "success", "message": "1 of 1 check passed", "counts": { "passed": 1, "failed": 0, "skipped": 0 } }, { "id": "cryptographic.proof", "phase": "cryptographic", - "suite": "proof", "status": "success", "hasFailures": false, + "suite": "proof", "status": "success", "message": "1 of 1 check passed", "counts": { "passed": 1, "failed": 0, "skipped": 0 } }, + // Non-fatal failure: status "failure", but no fatalFailureAt + // and the credential is still verified. Render as a warning. { "id": "trust.registry", "phase": "trust", - "suite": "registry", "status": "success", "hasFailures": false, - "message": "1 of 1 check passed", - "counts": { "passed": 1, "failed": 0, "skipped": 0 } } + "suite": "registry", "status": "failure", + "message": "1 of 1 check failed", + "counts": { "passed": 0, "failed": 1, "skipped": 0 } } ] }, { "verified": false, - "verifiableCredential": { /* bad VC */ }, + "verifiableCredential": { /* expired VC */ }, "recognizedProfile": "obv3p0.openbadge", "results": [ { "id": "cryptographic.core.context-exists", /* success */ }, @@ -557,34 +609,39 @@ dcc-transaction-service UI implementation. { "id": "cryptographic.core.credential-id", /* success */ }, { "id": "cryptographic.core.proof-exists", /* success */ }, { "id": "recognition.profile", /* success */ }, - { "id": "cryptographic.proof.signature", /* success */ }, + { "id": "cryptographic.proof.signature", + "suite": "proof", "check": "proof.signature", + "outcome": { "status": "failure", "problems": [ + { "type": "https://www.w3.org/TR/vc-data-model#CREDENTIAL_EXPIRED", + "title": "Credential Expired", + "detail": "..." } + ]}, + "fatal": true }, { "id": "trust.registry.issuer", "suite": "registry", "check": "registry.issuer", - "outcome": { "status": "failure", "problems": [ - { "type": "https://www.w3.org/TR/vc-data-model#ISSUER_NOT_REGISTERED", - "title": "Issuer Not Registered", - "detail": "Issuer DID not in any registry." } - ], - "payload": { "matches": [], "uncheckedRegistries": [] }}, + "outcome": { "status": "skipped", + "reason": "Not run: proof.signature failed" }, "fatal": false } ], "summary": [ { "id": "cryptographic.core", "phase": "cryptographic", - "suite": "core", "status": "success", "hasFailures": false, + "suite": "core", "status": "success", "message": "5 of 5 checks passed", "counts": { "passed": 5, "failed": 0, "skipped": 0 } }, { "id": "recognition", "phase": "recognition", - "suite": "recognition", "status": "success", "hasFailures": false, + "suite": "recognition", "status": "success", "message": "1 of 1 check passed", "counts": { "passed": 1, "failed": 0, "skipped": 0 } }, + // The fatal failure: the one summary with fatalFailureAt. { "id": "cryptographic.proof", "phase": "cryptographic", - "suite": "proof", "status": "success", "hasFailures": false, - "message": "1 of 1 check passed", - "counts": { "passed": 1, "failed": 0, "skipped": 0 } }, - { "id": "trust.registry", "phase": "trust", - "suite": "registry", "status": "failure", "hasFailures": true, + "suite": "proof", "status": "failure", "message": "1 of 1 check failed", - "counts": { "passed": 0, "failed": 1, "skipped": 0 } } + "counts": { "passed": 0, "failed": 1, "skipped": 0 }, + "fatalFailureAt": "cryptographic.proof.signature" }, + { "id": "trust.registry", "phase": "trust", + "suite": "registry", "status": "skipped", + "message": "registry not run: proof.signature failed", + "counts": { "passed": 0, "failed": 0, "skipped": 1 } } ] } ] @@ -606,7 +663,6 @@ interface SuiteSummary { phase: SuiteSummaryPhase; suite: string; status: 'success' | 'failure' | 'skipped' | 'mixed'; - hasFailures: boolean; message: string; counts: { passed: number; failed: number; skipped: number }; fatalFailureAt?: string; diff --git a/src/fold-results.ts b/src/fold-results.ts index 3374e92..9f9ff4b 100644 --- a/src/fold-results.ts +++ b/src/fold-results.ts @@ -113,7 +113,6 @@ function summarizeSuite( const fatalFailureAt = fatalCheck?.id; const status = deriveStatus(counts); - const hasFailures = counts.failed > 0; const message = formatMessage( suiteId, @@ -130,7 +129,6 @@ function summarizeSuite( phase: phase ?? 'unknown', suite: suiteId, status, - hasFailures, message, counts, ...(fatalFailureAt !== undefined ? { fatalFailureAt } : {}), diff --git a/src/types/result.ts b/src/types/result.ts index 6d2a276..f9c2970 100644 --- a/src/types/result.ts +++ b/src/types/result.ts @@ -5,7 +5,10 @@ import { VerifiableCredential } from '../schemas/credential.js'; import { VerifiablePresentation } from '../schemas/presentation.js'; export interface CredentialVerificationResult { - /** True if no check returned a failure outcome. */ + /** + * True when no fatal check failed. Non-fatal failures (warnings) + * leave this `true`; see {@link SuiteSummary.fatalFailureAt}. + */ verified: boolean; /** @@ -82,7 +85,10 @@ export interface CredentialVerificationResult { } export interface PresentationVerificationResult { - /** True if no failures in presentation checks or any credential checks. */ + /** + * True when no presentation-level check failed fatally and every + * entry in {@link credentialResults} is `verified`. + */ verified: boolean; /** diff --git a/src/types/suite-summary.ts b/src/types/suite-summary.ts index 1adf568..3484e77 100644 --- a/src/types/suite-summary.ts +++ b/src/types/suite-summary.ts @@ -57,25 +57,14 @@ export interface SuiteSummary { * - `'success'` — every check that ran returned `success`. * - `'failure'` — every check that ran returned `failure`. * - `'mixed'` — at least one `failure` and at least one - * non-`failure` (`success` or `skipped`). + * `success`. A suite whose only non-failures are skips is + * `'failure'`. * - `'skipped'` — no check ran (suite predicate returned false, * every check was filtered out by `appliesTo`, or every check * was halted by an earlier fatal failure). */ status: 'success' | 'failure' | 'skipped' | 'mixed'; - /** - * True iff at least one child check returned `'failure'`, - * whether or not that check was fatal. - * - * Deliberately not called `verified`: the only `verified` in a - * result is the top-level one, which means "no fatal check - * failed" across the whole subject. A suite with a non-fatal - * failure has `hasFailures: true` inside a `verified: true` - * result, and that is not a contradiction. - */ - hasFailures: boolean; - /** * Human-readable rollup message. Conventions: * - `" of checks passed"` @@ -95,10 +84,23 @@ export interface SuiteSummary { counts: { passed: number; failed: number; skipped: number }; /** - * Set when a fatal check failure halted the suite mid-run; - * carries the failing check's `id`. `counts.passed` and - * `counts.failed` reflect what actually executed before the - * halt; the halted checks count as `skipped`. + * The `id` of the check in this suite that failed fatally, if + * one did. A fatal failure halts every later check for the + * subject, so at most one suite per subject carries this field, + * and the subject's `verified` is `false` exactly when one of + * its summaries does. `counts.passed` and `counts.failed` + * reflect what actually executed before the halt; the halted + * checks count as `skipped`. + * + * `status` is computed from counts alone and does not say + * whether a failure was fatal: a suite whose only failure is + * non-fatal has `status: 'failure'` or `'mixed'` inside a + * `verified: true` result. Render such a suite as a warning, + * and reserve error styling for the suite with + * `fatalFailureAt`. + * + * There is deliberately no per-suite `verified`: the only + * verdict in a result is the top-level `verified`. */ fatalFailureAt?: string; diff --git a/src/verifier.ts b/src/verifier.ts index 532b900..d5861a6 100644 --- a/src/verifier.ts +++ b/src/verifier.ts @@ -435,8 +435,8 @@ function parseErrorResult( /** * Build the one-entry `summary[]` that accompanies every * parse-error result. The single check is always a fatal failure, - * so the summary always has `status: 'failure'`, - * `hasFailures: true`. + * so the summary always has `status: 'failure'` and + * `fatalFailureAt: 'cryptographic.parsing.envelope'`. * * `foldCheckResults` rolls per-check timing into the summary, so * the suite-level `timing` propagates here too when the call ran diff --git a/test/fold-results.spec.ts b/test/fold-results.spec.ts index 976a65c..5d9ce9c 100644 --- a/test/fold-results.spec.ts +++ b/test/fold-results.spec.ts @@ -136,7 +136,6 @@ describe('foldCheckResults', () => { ]); expect(summaries).toHaveLength(1); expect(summaries[0].status).toBe('success'); - expect(summaries[0].hasFailures).toBe(false); expect(summaries[0].counts).toEqual({ passed: 4, failed: 0, @@ -162,7 +161,8 @@ describe('foldCheckResults', () => { expect(results).toHaveLength(2); expect(summaries[0].status).toBe('failure'); - expect(summaries[0].hasFailures).toBe(true); + // Non-fatal failures: `status` reports them, `fatalFailureAt` does not. + expect(summaries[0].fatalFailureAt).toBeUndefined(); expect(summaries[0].counts).toEqual({ passed: 0, failed: 2, @@ -195,7 +195,7 @@ describe('foldCheckResults', () => { expect(results.filter(r => r.outcome.status === 'failure')).toHaveLength(1); expect(results[1].check).toBe('core.b'); expect(summaries[0].status).toBe('mixed'); - expect(summaries[0].hasFailures).toBe(true); + expect(summaries[0].fatalFailureAt).toBeUndefined(); expect(summaries[0].counts).toEqual({ passed: 3, failed: 1, @@ -219,7 +219,6 @@ describe('foldCheckResults', () => { expect(results).toHaveLength(1); expect(results[0].check).toBe('extra.applies'); expect(summaries[0].status).toBe('skipped'); - expect(summaries[0].hasFailures).toBe(false); expect(summaries[0].counts).toEqual({ passed: 0, failed: 0, @@ -257,7 +256,6 @@ describe('foldCheckResults', () => { 'proof.d' ]); expect(summaries[0].status).toBe('mixed'); - expect(summaries[0].hasFailures).toBe(true); expect(summaries[0].counts).toEqual({ passed: 1, failed: 1, @@ -317,7 +315,6 @@ describe('foldCheckResults', () => { 'proof not run: core.context-exists failed' ); expect(summaries[1].status).toBe('skipped'); - expect(summaries[1].hasFailures).toBe(false); expect(summaries[1].counts).toEqual({ passed: 0, failed: 0, diff --git a/test/types.spec.ts b/test/types.spec.ts index 47b27f3..ca942f5 100644 --- a/test/types.spec.ts +++ b/test/types.spec.ts @@ -110,7 +110,6 @@ describe('Foundation types', () => { phase: 'cryptographic', suite: 'core', status: 'success', - hasFailures: false, message: '4 of 4 checks passed', counts: { passed: 4, failed: 0, skipped: 0 } }; @@ -118,13 +117,12 @@ describe('Foundation types', () => { expect(summary.counts.passed).toBe(4); }); - it('records fatalFailureAt when the suite halted mid-run', () => { + it('records fatalFailureAt when a check in the suite failed fatally', () => { const summary: SuiteSummary = { id: 'cryptographic.proof', phase: 'cryptographic', suite: 'proof', status: 'failure', - hasFailures: true, message: '1 of 3 checks failed (0 passed)', counts: { passed: 0, failed: 1, skipped: 0 }, fatalFailureAt: 'proof.signature' @@ -139,7 +137,6 @@ describe('Foundation types', () => { phase, suite: 'custom', status: 'success', - hasFailures: false, message: '1 of 1 checks passed', counts: { passed: 1, failed: 0, skipped: 0 } }; @@ -155,7 +152,6 @@ describe('Foundation types', () => { phase: 'recognition', suite: 'recognition', status: 'skipped', - hasFailures: false, message: 'no recognizer matched', counts: { passed: 0, failed: 0, skipped: 1 } }; diff --git a/test/verify-credential.spec.ts b/test/verify-credential.spec.ts index 06dba30..49c90b3 100644 --- a/test/verify-credential.spec.ts +++ b/test/verify-credential.spec.ts @@ -282,6 +282,43 @@ describe('verifyCredential', () => { expect(result.verified).toBe(false); const customResult = result.results.find(r => r.suite === 'custom'); expect(customResult?.outcome.status).toBe('failure'); + const customSummary = result.summary.find(s => s.suite === 'custom'); + expect(customSummary?.fatalFailureAt).toBe('unknown.custom.fatal-check'); + }); + + it('a custom non-fatal failure fails its suite but leaves verified true', async () => { + const warningCheck: VerificationCheck = { + id: 'custom.warning-check', + name: 'Warning Check', + fatal: false, + execute: async (): Promise => ({ + status: 'failure', + problems: [ + { + type: 'https://www.w3.org/TR/vc-data-model#CUSTOM_ERROR', + title: 'Custom Warning', + detail: 'This custom check always fails' + } + ] + }) + }; + + const credential = CredentialFactory({ version: 'v1', credential: {} }); + const result = await verifyCredential({ + credential, + additionalSuites: [ + { id: 'custom', name: 'Custom Suite', checks: [warningCheck] } + ], + ...fakeVerified + }); + + expect(result.verified).toBe(true); + const customSummary = result.summary.find(s => s.suite === 'custom'); + expect(customSummary?.status).toBe('failure'); + expect(customSummary?.fatalFailureAt).toBeUndefined(); + expect(result.summary.some(s => s.fatalFailureAt !== undefined)).toBe( + false + ); }); it('a custom check that throws yields a CHECK_ERROR result, not a rejection', async () => { @@ -582,7 +619,7 @@ describe('verifyCredential', () => { }); expect(result.summary.length).toBeGreaterThan(0); - expect(result.summary.every(s => !s.hasFailures)).toBe(true); + expect(result.summary.every(s => s.counts.failed === 0)).toBe(true); const ids = new Set(result.summary.map(s => s.id)); expect(ids).toContain('cryptographic.core'); expect(ids).toContain('cryptographic.proof'); @@ -617,7 +654,9 @@ describe('verifyCredential', () => { ).toBe(true); const proofSummary = result.summary.find(s => s.suite === 'proof'); expect(['failure', 'mixed']).toContain(proofSummary?.status); - expect(proofSummary?.hasFailures).toBe(true); + expect(proofSummary?.fatalFailureAt).toBe( + 'cryptographic.proof.signature' + ); }); it('parse error: result has one summary entry tagged cryptographic.parsing', async () => { diff --git a/test/verify-presentation.spec.ts b/test/verify-presentation.spec.ts index a60b035..aa3f58f 100644 --- a/test/verify-presentation.spec.ts +++ b/test/verify-presentation.spec.ts @@ -463,14 +463,14 @@ describe('verifyPresentation', () => { true ); expect(result.summary.length).toBeGreaterThan(0); - expect(result.summary.every(s => !s.hasFailures)).toBe(true); + expect(result.summary.every(s => s.counts.failed === 0)).toBe(true); expect(result.credentialResults).toHaveLength(1); const cred = result.credentialResults[0]; expect(cred.results.length).toBeGreaterThan(0); expect(cred.results.every(r => r.id !== undefined)).toBe(true); expect(cred.summary.length).toBeGreaterThan(0); - expect(cred.summary.every(s => !s.hasFailures)).toBe(true); + expect(cred.summary.every(s => s.counts.failed === 0)).toBe(true); }); }); @@ -502,7 +502,7 @@ describe('verifyPresentation', () => { const result = await verifyPresentation({ presentation, ...cryptoOnly }); expect(result.summary.length).toBeGreaterThan(0); - expect(result.summary.every(s => !s.hasFailures)).toBe(true); + expect(result.summary.every(s => s.counts.failed === 0)).toBe(true); }); it('passing credential summary is all green; failing credential surfaces a failure entry', async () => { @@ -514,14 +514,23 @@ describe('verifyPresentation', () => { expect(passing).toBeDefined(); expect(failing).toBeDefined(); - expect(passing!.summary.every(s => !s.hasFailures)).toBe(true); + expect(passing!.summary.every(s => s.counts.failed === 0)).toBe(true); expect(passing!.results.length).toBeGreaterThan(0); expect( passing!.results.every(r => r.outcome.status !== 'failure') ).toBe(true); - const failureSummaries = failing!.summary.filter(s => s.hasFailures); + const failureSummaries = failing!.summary.filter( + s => s.counts.failed > 0 + ); expect(failureSummaries.length).toBeGreaterThan(0); + // The fatal failure that made it unverified is on exactly one summary. + expect( + failing!.summary.filter(s => s.fatalFailureAt !== undefined) + ).toHaveLength(1); + expect(passing!.summary.some(s => s.fatalFailureAt !== undefined)).toBe( + false + ); expect( failing!.results.filter(r => r.outcome.status === 'failure').length ).toBeGreaterThan(0); @@ -532,7 +541,7 @@ describe('verifyPresentation', () => { const result = await verifyPresentation({ presentation, ...cryptoOnly }); const failing = result.credentialResults.find(c => !c.verified)!; - const failingSummary = failing.summary.find(s => s.hasFailures)!; + const failingSummary = failing.summary.find(s => s.counts.failed > 0)!; // `results[]` carries passes and skips too, so a prefix match // has to be narrowed by status to reach the failure detail. const detail = failing.results.filter(