From c0a7a6eb55ccf9748b1c605809f96099d90fcd94 Mon Sep 17 00:00:00 2001 From: Tim Nunamaker Date: Thu, 5 Nov 2026 13:38:01 -0600 Subject: [PATCH 1/9] feat(runtime): accept OBSERVATION messages and keep a per-run diagnostic record Collection Profile 0.2.0 lets a connector report what it saw with OBSERVATION messages (Section 5.10), and lets a failed DONE cite them with `error.basis` (Section 5.8). The runtime had no handler, so START did not advertise the capability and a connector could not use it. The runtime now: - advertises OBSERVATION in START and validates the manifest's profile declaration (protocol_version, capabilities, diagnostic_descriptors) before spawn; - accepts or rejects each OBSERVATION without failing the run. A rejection is recorded as a runtime defect. Raw OBSERVATION lines never reach the trace; - applies the strict DONE error shape to a 0.2.0 manifest: failed DONE needs `error` with a boolean `retryable`, `cancelled` is not a connector status, and `basis` may cite only ids the connector emitted. An invalid DONE keeps its bounded connector message visible; - stores one bounded diagnostic record per run (observation snapshot and diagnosis, 8 runs per connector) in `connector_observation_runs`, on SQLite and PostgreSQL, and serves it to the owner session at `/_ref/runs/:run_id/observations`. The Section 5.11 recovery-hint gate is implemented in `observations.ts` and covered by its unit tests. Wiring it to DONE and SKIP_RESULT hints changes retry behavior and lands separately. Assisted-by: AI Signed-off-by: Tim Nunamaker --- PG-PROFILE-51-REPORT.md | 2 +- .../runtime/collection-profile-support.ts | 61 ++ .../runtime/done-validators.ts | 69 +- reference-implementation/runtime/index.ts | 161 +++- .../runtime/observation.schema.json | 256 ++++++ .../runtime/observations.ts | 713 +++++++++++++++ .../scripts/postgres-template-eligibility.ts | 5 + .../server/backup-table-policy.ts | 5 + reference-implementation/server/db.ts | 21 + reference-implementation/server/index.ts | 7 + .../server/postgres-storage.ts | 17 + .../server/routes/ref-run-observations.ts | 79 ++ .../stores/connector-observation-store.ts | 448 ++++++++++ .../test/collection-profile-020.test.ts | 615 +++++++++++++ .../collection-profile-owner-journey.test.ts | 108 +++ .../test/collection-profile.test.ts | 41 +- .../test/connector-observation-store.test.ts | 843 ++++++++++++++++++ ...detail-coverage-shortfall-severity.test.ts | 9 +- .../done-error-normalization-oracle.test.ts | 11 + ...zero-connector-knowledge-data-load-scan.ts | 14 +- .../test/ref-run-observations-route.test.ts | 190 ++++ ...ro-connector-knowledge-conformance.test.ts | 30 + .../test/runtime-observations.test.ts | 791 ++++++++++++++++ 23 files changed, 4450 insertions(+), 46 deletions(-) create mode 100644 reference-implementation/runtime/collection-profile-support.ts create mode 100644 reference-implementation/runtime/observation.schema.json create mode 100644 reference-implementation/runtime/observations.ts create mode 100644 reference-implementation/server/routes/ref-run-observations.ts create mode 100644 reference-implementation/server/stores/connector-observation-store.ts create mode 100644 reference-implementation/test/collection-profile-020.test.ts create mode 100644 reference-implementation/test/collection-profile-owner-journey.test.ts create mode 100644 reference-implementation/test/connector-observation-store.test.ts create mode 100644 reference-implementation/test/ref-run-observations-route.test.ts create mode 100644 reference-implementation/test/runtime-observations.test.ts diff --git a/PG-PROFILE-51-REPORT.md b/PG-PROFILE-51-REPORT.md index ec4534cf9..1c29ee3c9 100644 --- a/PG-PROFILE-51-REPORT.md +++ b/PG-PROFILE-51-REPORT.md @@ -168,7 +168,7 @@ tail: The completed profile receipt recorded 1,034 files; that historical receipt is unchanged. The template equivalence registry is a different measure: -156 eligible plus 37 cold-required files, for **193** total. The +159 eligible plus 37 cold-required files, for **196** total. The handle-generation, issuance-cutover, opaque-cursor and search-index repair tests remain registered. These counts are recomputed from the merged registry arrays. diff --git a/reference-implementation/runtime/collection-profile-support.ts b/reference-implementation/runtime/collection-profile-support.ts new file mode 100644 index 000000000..a00996ed4 --- /dev/null +++ b/reference-implementation/runtime/collection-profile-support.ts @@ -0,0 +1,61 @@ +// Copyright The PDP-Connect Contributors +// SPDX-License-Identifier: Apache-2.0 + +// Admission of a manifest's Collection Profile declaration on the run path +// (runtime/index.ts). +// +// Normative source: data-connectors docs spec sections 3.4 (protocol_version +// and capabilities) and 3.8 (diagnostic_descriptors). A manifest that declares +// diagnostic_descriptors or OBSERVATION declares protocol_version 0.2.0. + +import { + COLLECTION_PROFILE_VERSIONS, + collectionProfileDeclarationError, + OBSERVATION_CAPABILITY, +} from "@pdpp/connector-protocol/connector-runtime-protocol"; +import { type DiagnosticDescriptors, ObservationContractError, validateDiagnosticDescriptors } from "./observations.ts"; + +export interface CollectionProfileDeclaration { + readonly descriptors: DiagnosticDescriptors; + readonly observationDeclared: boolean; +} + +/** + * Returns the first problem with the declaration, as a bounded message, or + * null when it is admissible. The message names the rule, never the + * manifest's value. + */ +export function collectionProfileDeclarationProblem(manifest: Record): string | null { + const version = manifest.protocol_version === undefined ? "0.1.0" : manifest.protocol_version; + if (!(COLLECTION_PROFILE_VERSIONS as readonly unknown[]).includes(version)) { + return "protocol_version must be one of 0.1.0, 0.2.0"; + } + const capabilityProblem = collectionProfileDeclarationError(version, manifest.protocol_capabilities); + if (capabilityProblem !== null) { + return capabilityProblem; + } + if (manifest.diagnostic_descriptors !== undefined && version !== "0.2.0") { + return "diagnostic_descriptors require protocol_version 0.2.0"; + } + try { + validateDiagnosticDescriptors(manifest.diagnostic_descriptors); + } catch (error) { + if (error instanceof ObservationContractError) { + return `diagnostic_descriptors rejected: ${error.code}`; + } + throw error; + } + return null; +} + +/** + * Reads an admitted declaration. Call only after `collectionProfileDeclarationProblem` + * returns null; descriptor validation has then already passed. + */ +export function readCollectionProfileDeclaration(manifest: Record): CollectionProfileDeclaration { + const capabilities = Array.isArray(manifest.protocol_capabilities) ? manifest.protocol_capabilities : []; + return { + descriptors: validateDiagnosticDescriptors(manifest.diagnostic_descriptors), + observationDeclared: capabilities.includes(OBSERVATION_CAPABILITY), + }; +} diff --git a/reference-implementation/runtime/done-validators.ts b/reference-implementation/runtime/done-validators.ts index 083998d5e..742b9cc79 100644 --- a/reference-implementation/runtime/done-validators.ts +++ b/reference-implementation/runtime/done-validators.ts @@ -15,6 +15,7 @@ // runtime state, secret handling, or grant/scope enforcement. import { isValidRecoveryHintShape } from "./connector-gap-bounding.ts"; +import { isObservationIdSyntax } from "./observations.ts"; type DoneStatus = "succeeded" | "failed" | "cancelled"; @@ -23,14 +24,16 @@ interface DoneErrorInput { message?: string; recovery_hint?: unknown; retryable?: boolean | null; + basis?: unknown; [key: string]: unknown; } -interface DoneError { +export interface DoneError { code?: string; message: string; recovery_hint?: string | { action: string; retryable?: boolean }; retryable: boolean | null; + basis?: readonly string[]; } interface DoneMessage { @@ -71,8 +74,8 @@ export function validateDoneRecordsEmitted( return null; } -export function validateDoneStatus(status: unknown): Error | null { - if (status === "succeeded" || status === "failed" || status === "cancelled") { +export function validateDoneStatus(status: unknown, portable = false): Error | null { + if (status === "succeeded" || status === "failed" || (!portable && status === "cancelled")) { return null; } return new Error(`Connector emitted invalid DONE status: ${status}`); @@ -80,12 +83,34 @@ export function validateDoneStatus(status: unknown): Error | null { const DONE_ERROR_CODE_RE = /^[a-z][a-z0-9_]{0,63}$/; +/** + * What the run's protocol state lets a DONE error say. `strict` is the + * Collection Profile 0.2.0 shape: a failed DONE carries an error and a boolean + * `retryable`. The legacy default keeps the 0.1 shape. + */ +export interface DoneErrorContract { + readonly strict: boolean; + readonly observationDeclared: boolean; + /** True when the connector emitted this OBSERVATION id earlier in the run. */ + readonly hasEmittedId: (id: string) => boolean; +} + +const LEGACY_DONE_ERROR_CONTRACT: DoneErrorContract = { + strict: false, + observationDeclared: false, + hasEmittedId: () => false, +}; + export function validateDoneError( status: DoneStatus | unknown, - error: DoneErrorInput | null | undefined + error: DoneErrorInput | null | undefined, + contract: DoneErrorContract = LEGACY_DONE_ERROR_CONTRACT ): DoneError | Error | null { // biome-ignore lint/suspicious/noEqualsToNull: check for both null and undefined if (error == null) { + if (contract.strict && status === "failed") { + return new Error("Connector emitted invalid DONE: failed status requires DONE.error"); + } return null; } if (status === "succeeded") { @@ -95,7 +120,8 @@ export function validateDoneError( return new Error("Connector emitted invalid DONE.error: expected object"); } const unsupportedFields = Object.keys(error).filter( - (field) => field !== "message" && field !== "retryable" && field !== "code" && field !== "recovery_hint" + (field) => + field !== "message" && field !== "retryable" && field !== "code" && field !== "recovery_hint" && field !== "basis" ); if (unsupportedFields.length) { return new Error(`Connector emitted invalid DONE.error: unsupported fields ${unsupportedFields.join(", ")}`); @@ -107,6 +133,9 @@ export function validateDoneError( if (typeof error.message !== "string" || !error.message.trim()) { return new Error("Connector emitted invalid DONE.error.message: expected non-empty string"); } + if (contract.strict && typeof error.retryable !== "boolean") { + return new Error("Connector emitted invalid DONE.error.retryable: expected boolean"); + } // biome-ignore lint/suspicious/noEqualsToNull: check for both null and undefined if (error.retryable != null && typeof error.retryable !== "boolean") { return new Error("Connector emitted invalid DONE.error.retryable: expected boolean"); @@ -114,6 +143,10 @@ export function validateDoneError( if (!isValidRecoveryHintShape(error.recovery_hint)) { return new Error("Connector emitted invalid DONE.error.recovery_hint"); } + const basis = validateDoneBasis(error.basis, contract); + if (basis instanceof Error) { + return basis; + } return { ...(error.code ? { code: error.code } : {}), message: error.message.trim(), @@ -122,5 +155,31 @@ export function validateDoneError( ? {} : { recovery_hint: error.recovery_hint as NonNullable }), retryable: error.retryable ?? null, + ...(basis === undefined ? {} : { basis }), }; } + +/** + * Section 5.8: `basis` is OPTIONAL, needs a declared OBSERVATION capability, + * and cites only ids the connector emitted in this run. Invalid-but-emitted + * ids count as emitted. Returns the ids, undefined when absent, or an Error. + */ +function validateDoneBasis(basis: unknown, contract: DoneErrorContract): readonly string[] | undefined | Error { + if (basis === undefined) { + return undefined; + } + if (!contract.observationDeclared) { + return new Error("Connector emitted DONE.error.basis without declaring OBSERVATION"); + } + if (!Array.isArray(basis)) { + return new Error("Connector emitted invalid DONE.error.basis: expected array of observation ids"); + } + for (const id of basis) { + if (!(isObservationIdSyntax(id) && contract.hasEmittedId(id))) { + return new Error("Connector emitted DONE.error.basis citing an OBSERVATION id it did not emit"); + } + } + // The emitted-id ledger bounds unique citations. Duplicate citations add no + // diagnosis authority and need not expand the stored record. + return [...new Set(basis)] as string[]; +} diff --git a/reference-implementation/runtime/index.ts b/reference-implementation/runtime/index.ts index 4b5d9ffdc..dd2531ef6 100644 --- a/reference-implementation/runtime/index.ts +++ b/reference-implementation/runtime/index.ts @@ -19,6 +19,7 @@ import { DETAIL_COVERAGE_KEY_SETS_CAPABILITY, HOST_BLOB_CAPABILITY, isConnectorProtocolCapabilityArray, + OBSERVATION_CAPABILITY, PERMANENT_DETAIL_GAP_REASON, RETRYABLE_DETAIL_GAP_REASONS, STREAM_EVIDENCE_CAPABILITY, @@ -34,6 +35,7 @@ import { resolveRunConnectorOptions } from "../server/connector-run-config.ts"; import { readStoredCollectionScope } from "../server/local-collection-scope.ts"; import { encodeKey } from "../server/records.ts"; import { getDefaultCheckpointWithholdStore } from "../server/stores/checkpoint-withhold-store.ts"; +import { getDefaultConnectorObservationStore } from "../server/stores/connector-observation-store.ts"; import { getDefaultConnectorAttentionStore } from "../server/stores/connector-attention-store.ts"; import { getDefaultConnectorDetailGapStore } from "../server/stores/connector-detail-gap-store.ts"; import type { ConnectorInstanceConfigStore } from "../server/stores/connector-instance-config-store.ts"; @@ -91,6 +93,11 @@ import { cursorAuditFact } from "./cursor-audit-fact.ts"; import { describeCursorBandViolation, evaluateCursorBand } from "./cursor-band-contiguity.ts"; import { declaredReasonTokensFor } from "./declared-reason-tokens.ts"; import { createDetailGapPageReader, validateDetailGapsPageRequest } from "./detail-gap-paging.ts"; +import { + collectionProfileDeclarationProblem, + readCollectionProfileDeclaration, +} from "./collection-profile-support.ts"; +import type { DoneError } from "./done-validators.ts"; import { validateDoneError, validateDoneExitCode, @@ -99,6 +106,7 @@ import { } from "./done-validators.ts"; import type { IngestResult as StrictIngestResult } from "./ingest-failure.ts"; import { readIngestResponse as strictReadIngestResponse } from "./ingest-failure.ts"; +import { createObservationRun } from "./observations.ts"; import { buildHttpFailure, buildIngestEnvelopeContractViolationFailure, @@ -2595,14 +2603,24 @@ export function buildBrowserSurfaceLaunchEnv({ /** * Protocol capabilities this runtime implements, listed in * START.protocol_capabilities. It accepts DETAIL_COVERAGE empty_keys and - * unchanged_keys, and validates STREAM_EVIDENCE. It has no BLOB transport and - * no OBSERVATION handler, so it does not list them. + * unchanged_keys, validates STREAM_EVIDENCE and accepts OBSERVATION (Section + * 5.10). It has no BLOB transport, so it does not list it. */ const RUNTIME_PROTOCOL_CAPABILITIES: readonly ConnectorProtocolCapability[] = [ DETAIL_COVERAGE_KEY_SETS_CAPABILITY, + OBSERVATION_CAPABILITY, STREAM_EVIDENCE_CAPABILITY, ]; +/** Type of a Collection Profile 0.2.0 OBSERVATION message (Section 5.10). */ +const OBSERVATION_MESSAGE_TYPE = "OBSERVATION"; +/** An unparseable line is an OBSERVATION only when its text names the type. */ +const OBSERVATION_TYPE_TEXT_RE = /"type"\s*:\s*"OBSERVATION"/; + +function isObservationMessage(value: unknown): boolean { + return typeof value === "object" && value !== null && (value as { type?: unknown }).type === OBSERVATION_MESSAGE_TYPE; +} + /** * Declared capabilities this runtime starts a connector with although it does * not implement them. BLOB is a runtime-specific extension (Collection Profile @@ -3058,7 +3076,20 @@ export async function runConnector(opts: RuntimeRunConnectorOptions): Promise); + if (declarationProblem !== null) { + throw new Error(`Connector manifest ${declarationProblem}`); + } validateRequiredProtocolCapabilities(manifest); + const profileDeclaration = readCollectionProfileDeclaration(manifest as unknown as Record); + // A 0.2.0 manifest gets the strict DONE error shape (Section 5.8). + const strictDoneError = manifest.protocol_version === "0.2.0"; + const observationRun = createObservationRun({ + declared: profileDeclaration.observationDeclared, + descriptors: profileDeclaration.descriptors, + }); + // Byte counts and parsed values belong to the transport, never wire fields. + const observationInputs = new WeakMap(); const explicitlyRequestedStreams = requestedRuntimeStreams(providedScope); const declaredCollectionScope = readStoredCollectionScope(state).scope; @@ -3692,6 +3723,8 @@ export async function runConnector(opts: RuntimeRunConnectorOptions): Promise { cleanupChildHandles(); + await persistObservationDiagnosis(); try { await awaitLeaseAccounting(); } catch (accountingErr) { @@ -5260,8 +5294,9 @@ export async function runConnector(opts: RuntimeRunConnectorOptions): Promise { - const invalidDoneStatus = validateDoneStatus(msg.status); + const invalidDoneStatus = validateDoneStatus(msg.status, strictDoneError); if (invalidDoneStatus) { - throw invalidDoneStatus; + throw invalidDoneEnvelope(invalidDoneStatus, msg); } const doneStatus = msg.status as DoneMessageState["status"]; - const normalizedDoneError = validateDoneError(doneStatus, msg.error as Parameters[1]); - if (normalizedDoneError instanceof Error) { - throw normalizedDoneError; + const validatedDoneError = validateDoneError(doneStatus, msg.error as Parameters[1], { + strict: strictDoneError, + observationDeclared: profileDeclaration.observationDeclared, + hasEmittedId: (id) => observationRun.hasEmittedId(id), + }); + if (validatedDoneError instanceof Error) { + throw invalidDoneEnvelope(validatedDoneError, msg); } + const normalizedDoneError = validatedDoneError === null ? null : withoutDoneBasis(validatedDoneError); + doneBasis = validatedDoneError?.basis ?? []; finalStatus = doneStatus; doneMessage = { error: normalizedDoneError as ConnectorDoneError | null, @@ -6470,6 +6511,7 @@ export async function runConnector(opts: RuntimeRunConnectorOptions): Promise { + // Accept, or reject and continue (Section 5.10). A rejection is recorded + // as a runtime defect and never fails the run. The raw line is not kept. + const input = observationInputs.get(msg); + if (!input) throw new Error("Missing runtime observation envelope"); + observationInputs.delete(msg); + observationRun.accept(input.value, input.serializedBytes); + return Promise.resolve(); + } + + let observationDiagnosisWrite: Promise | null = null; + + /** + * The owner diagnostic record (Section 5.10 and 5.11), written once per run. + * The first settle point starts it, and every settle point awaits it, so the + * record exists before the caller's promise settles. I/O errors are reported as + * progress and never change the run's outcome. + */ + function persistObservationDiagnosis(): Promise { + observationDiagnosisWrite ??= getDefaultConnectorObservationStore() + .put({ + connectorId, + connectorInstanceId: resolvedConnectorInstanceId, + diagnosis: observationRun.diagnose(doneBasis), + runId, + snapshot: observationRun.snapshot(), + }) + .then( + () => undefined, + () => { + // Optional reporting must not turn a diagnostic write failure into + // a collection failure if the consumer also rejects progress. + try { + onProgress({ type: "observation_record_error" }); + } catch { + // Keep the collection outcome. + } + } + ); + return observationDiagnosisWrite; + } + + async function rejectAfterDiagnosis(error: unknown): Promise { + await persistObservationDiagnosis(); + reject(error); + } + async function handleMsg(msg: ConnectorMessage): Promise { if (doneMessage) { if (msg.type === "__PARSE_ERROR__") { @@ -6497,12 +6602,38 @@ export async function runConnector(opts: RuntimeRunConnectorOptions): Promise { - writeTrace(line); + // Parse once. A line is an OBSERVATION by its parsed type, or by its text + // when it does not parse. Raw OBSERVATION lines are never traced, logged, + // spined or webhooked (Section 5.10). + let parsed: unknown; + let parseError: RuntimeRunError | null = null; + try { + parsed = JSON.parse(line); + } catch (caught) { + parseError = caught as RuntimeRunError; + } + const observation = parseError === null ? isObservationMessage(parsed) : OBSERVATION_TYPE_TEXT_RE.test(line); + if (!observation) { + writeTrace(line); + } if (!line.trim()) { return; } + if (observation) { + if (failPendingInteraction(new Error("Connector emitted OBSERVATION while waiting for INTERACTION_RESPONSE"))) { + return; + } + const envelope: ConnectorMessage = { type: OBSERVATION_MESSAGE_TYPE }; + observationInputs.set(envelope, { value: parsed, serializedBytes: Buffer.byteLength(line, "utf8") }); + msgQueue.push(envelope); + processNext().catch(rejectAfterDiagnosis); + return; + } try { - const msg = JSON.parse(line); + if (parseError !== null) { + throw parseError; + } + const msg = parsed as ConnectorMessage; if (resolvePendingOtpInteraction(msg)) { // The matching resolution releases the protocol gate immediately. // DONE can arrive on the next line before the async interaction @@ -6513,7 +6644,7 @@ export async function runConnector(opts: RuntimeRunConnectorOptions): Promise | null ): Promise { cleanupChildHandles(); + await persistObservationDiagnosis(); await awaitLeaseAccounting(); const resolution = deriveClosedRunResolution(code); resolve(buildClosedRunResult({ code, resolution, stderrTailDiagnostic })); @@ -7133,6 +7265,7 @@ export async function runConnector(opts: RuntimeRunConnectorOptions): Promise reject(error)); diff --git a/reference-implementation/runtime/observation.schema.json b/reference-implementation/runtime/observation.schema.json new file mode 100644 index 000000000..8d9384d1f --- /dev/null +++ b/reference-implementation/runtime/observation.schema.json @@ -0,0 +1,256 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://github.com/PDP-Connect/data-connectors/schemas/observation.schema.json", + "title": "Collection Profile OBSERVATION message", + "description": "Machine-readable half of docs/spec/collection-profile.md Section 5.10 (the connector OBSERVATION message) and Section 3.8 (the manifest diagnostic_descriptors member, under $defs). The schema does not check that element_expectation.final is one of states_seen, that ids are unique within a run or a descriptor list, or that a fact names a declared id; the runtime checks those.", + "$ref": "#/$defs/observation", + "$defs": { + "descriptorId": { + "type": "string", + "pattern": "^[a-z][a-z0-9_]{0,63}$" + }, + "observationId": { + "type": "string", + "pattern": "^[A-Za-z0-9][A-Za-z0-9_.-]{0,63}$" + }, + "evidenceRef": { + "description": "A local artifact reference: printable ASCII without spaces, never a URL.", + "type": "string", + "pattern": "^(?!.*://)[!-~]{1,256}$" + }, + "elementState": { + "enum": [ + "matched", + "absent", + "hidden", + "disabled", + "ambiguous" + ] + }, + "resolves": { + "type": "array", + "items": { + "$ref": "#/$defs/observationId" + }, + "minItems": 1, + "maxItems": 16 + }, + "observation": { + "description": "One connector fact. Runtime-only fact types (credential_submission, rule_match, connector_defect) are not valid here.", + "type": "object", + "required": [ + "type", + "id", + "fact", + "attrs" + ], + "properties": { + "type": { + "const": "OBSERVATION" + }, + "id": { + "$ref": "#/$defs/observationId" + }, + "fact": { + "enum": [ + "element_expectation", + "wait_expired", + "provider_message" + ] + }, + "step": { + "$ref": "#/$defs/descriptorId" + }, + "attrs": { + "type": "object" + }, + "evidence_ref": { + "$ref": "#/$defs/evidenceRef" + }, + "resolves": { + "$ref": "#/$defs/resolves" + } + }, + "additionalProperties": false, + "oneOf": [ + { + "$ref": "#/$defs/elementExpectation" + }, + { + "$ref": "#/$defs/waitExpired" + }, + { + "$ref": "#/$defs/providerMessage" + } + ] + }, + "elementExpectation": { + "type": "object", + "required": [ + "step" + ], + "properties": { + "fact": { + "const": "element_expectation" + }, + "step": { + "$ref": "#/$defs/descriptorId" + }, + "attrs": { + "type": "object", + "required": [ + "expectation", + "states_seen", + "final" + ], + "properties": { + "expectation": { + "$ref": "#/$defs/descriptorId" + }, + "states_seen": { + "type": "array", + "items": { + "$ref": "#/$defs/elementState" + }, + "minItems": 1, + "maxItems": 8 + }, + "final": { + "$ref": "#/$defs/elementState" + }, + "fallback_used": { + "type": "boolean" + } + }, + "additionalProperties": false + } + } + }, + "waitExpired": { + "type": "object", + "required": [ + "step" + ], + "properties": { + "fact": { + "const": "wait_expired" + }, + "step": { + "$ref": "#/$defs/descriptorId" + }, + "attrs": { + "type": "object", + "required": [ + "awaited", + "budget_ms" + ], + "properties": { + "awaited": { + "$ref": "#/$defs/descriptorId" + }, + "budget_ms": { + "type": "integer", + "minimum": 0, + "maximum": 86400000 + } + }, + "additionalProperties": false + } + } + }, + "providerMessage": { + "type": "object", + "properties": { + "fact": { + "const": "provider_message" + }, + "attrs": { + "type": "object", + "required": [ + "kind" + ], + "properties": { + "kind": { + "enum": [ + "auth_failure", + "other" + ] + }, + "rule": { + "$ref": "#/$defs/descriptorId" + } + }, + "additionalProperties": false + } + } + }, + "diagnosticDescriptors": { + "description": "The manifest diagnostic_descriptors member (Section 3.8).", + "type": "object", + "properties": { + "steps": { + "type": "array", + "maxItems": 64, + "items": { + "type": "object", + "required": [ + "id" + ], + "properties": { + "id": { + "$ref": "#/$defs/descriptorId" + } + }, + "additionalProperties": false + } + }, + "expectations": { + "type": "array", + "maxItems": 256, + "items": { + "type": "object", + "required": [ + "id", + "step" + ], + "properties": { + "id": { + "$ref": "#/$defs/descriptorId" + }, + "step": { + "$ref": "#/$defs/descriptorId" + } + }, + "additionalProperties": false + } + }, + "rules": { + "type": "array", + "maxItems": 64, + "items": { + "type": "object", + "required": [ + "id", + "kind" + ], + "properties": { + "id": { + "$ref": "#/$defs/descriptorId" + }, + "kind": { + "enum": [ + "auth" + ] + }, + "step": { + "$ref": "#/$defs/descriptorId" + } + }, + "additionalProperties": false + } + } + }, + "additionalProperties": false + } + } +} diff --git a/reference-implementation/runtime/observations.ts b/reference-implementation/runtime/observations.ts new file mode 100644 index 000000000..708fb4076 --- /dev/null +++ b/reference-implementation/runtime/observations.ts @@ -0,0 +1,713 @@ +// Copyright The PDP-Connect Contributors +// SPDX-License-Identifier: Apache-2.0 + +// OBSERVATION intake, descriptor validation and failure diagnosis for the +// Collection Profile v0.2.0. +// +// Normative sources: data-connectors docs spec sections 3.8 (diagnostic +// descriptors), 5.10 (OBSERVATION facts, provenance, retirement, limits) and +// 5.11 (diagnosis and the recovery-hint gate). The wire grammar is the +// canonical JSON Schema `observation.schema.json` beside this file, a byte +// copy of data-connectors blob 8d9384d1f0827647e62e1914e8cac81d809034b3. The +// schema cannot express the checks below, so this module adds them: declared +// id references, final-in-states_seen, duplicate ids, the run limits and the +// diagnosis rules. +// +// The module returns data. It renders no text, retries nothing, prompts for no +// credential, and performs no provider or storage I/O. + +import { Ajv2020, type ValidateFunction } from "ajv/dist/2020.js"; + +import observationSchema from "./observation.schema.json" with { type: "json" }; + +// --------------------------------------------------------------------------- +// Public types +// --------------------------------------------------------------------------- + +export type ElementState = "matched" | "absent" | "hidden" | "disabled" | "ambiguous"; + +export interface DiagnosticDescriptors { + readonly expectations: readonly { + readonly id: string; + readonly step: string; + }[]; + readonly rules: readonly { + readonly id: string; + readonly kind: "auth"; + readonly step?: string; + }[]; + readonly steps: readonly { readonly id: string }[]; +} + +interface FactEnvelope { + readonly evidence_ref?: string; + readonly id: string; + readonly resolves?: readonly string[]; + readonly step?: string; + readonly type: "OBSERVATION"; +} + +export interface ElementExpectationFact extends FactEnvelope { + readonly attrs: { + readonly expectation: string; + readonly states_seen: readonly ElementState[]; + readonly final: ElementState; + readonly fallback_used?: boolean; + }; + readonly fact: "element_expectation"; + readonly step: string; +} + +export interface WaitExpiredFact extends FactEnvelope { + readonly attrs: { readonly awaited: string; readonly budget_ms: number }; + readonly fact: "wait_expired"; + readonly step: string; +} + +export interface ProviderMessageFact extends FactEnvelope { + readonly attrs: { + readonly kind: "auth_failure" | "other"; + readonly rule?: string; + }; + readonly fact: "provider_message"; +} + +export type ConnectorFact = ElementExpectationFact | WaitExpiredFact | ProviderMessageFact; + +export type ObservedFact = ConnectorFact & { readonly retired: boolean }; + +/** Reasons a connector OBSERVATION is discarded. This is the closed set of Section 5.10.2. */ +export type ObservationRejectReason = + | "runtime_fact_type" + | "undeclared_capability" + | "invalid" + | "duplicate_id" + | "oversize" + | "count_exceeded"; + +export type AcceptOutcome = + | { + readonly status: "accepted"; + readonly id: string; + readonly retired: readonly string[]; + } + | { readonly status: "rejected"; readonly reason: ObservationRejectReason }; + +/** + * Runtime-authored defect aggregate. It carries no connector text. Protocol + * violations aggregate by reason, so the record set is bounded by the closed + * reason list (seven records at most), whatever a connector sends. + */ +export type RuntimeDefect = + | { + readonly id: string; + readonly class: "protocol_violation"; + readonly reason: ObservationRejectReason; + readonly count: number; + } + | { + readonly id: string; + readonly class: "hint_unsupported"; + readonly count: number; + }; + +/** The portable recovery actions of Collection Profile Section 5.5. */ +export type PortableRecoveryAction = + | "retry_by_runtime" + | "retry_on_connector_upgrade" + | "refresh_credentials" + | "manual_action_required" + | "update_selector" + | "upstream_unblock" + | "not_retriable" + | "unknown"; + +/** Where a connector hint came from. */ +export type RecoveryHintOrigin = "skip_result" | "done_error"; + +/** + * A connector hint the Section 5.11 gate did not authorize. It is shown only + * as the connector's attributed suggestion. The record holds closed values + * only: no free text, code, or retryable flag from the connector. + */ +export interface ConnectorSuggestion { + readonly action: PortableRecoveryAction; + readonly attribution: "connector"; + readonly origin: RecoveryHintOrigin; +} + +export interface ObservationSnapshot { + /** Present only when at least one hint was not authorized. */ + readonly connector_suggestions?: readonly ConnectorSuggestion[]; + readonly defects: readonly RuntimeDefect[]; + readonly facts: readonly ObservedFact[]; + readonly truncated: boolean; +} + +export type DiagnosisCategory = "auth_rejected" | "expectation_mismatch" | "unknown"; + +export interface Diagnosis { + readonly attribution: "connector" | "none"; + /** Ids of the in-scope facts that support the category, in record order. */ + readonly basis: readonly string[]; + readonly category: DiagnosisCategory; + /** Other in-scope supporting facts, in record order. */ + readonly contributing: readonly string[]; +} + +export type HintGateOutcome = + | { readonly result: "authorized" } + | { readonly result: "blocked"; readonly reason: "truncated" } + | { readonly result: "unsupported" }; + +export interface ObservationLimits { + /** Serialized bytes per OBSERVATION. Must be at least 2048. */ + readonly maxFactBytes?: number; + /** Accepted connector facts per run. Must be at least 64. */ + readonly maxFacts?: number; + /** Connector OBSERVATION messages that reach validation per window. */ + readonly maxPerWindow?: number; + readonly windowMs?: number; +} + +export interface CreateObservationRunInput { + readonly declared: boolean; + readonly descriptors: DiagnosticDescriptors; + readonly limits?: ObservationLimits; + readonly now?: () => number; +} + +export interface ObservationRun { + /** Runtime-owned raw byte count, or canonical JSON size for in-memory callers. */ + accept: (value: unknown, serializedBytes?: number) => AcceptOutcome; + /** + * Intake for one raw wire line. The byte bound is the runtime's own UTF-8 + * count of that line, taken before any parse. Nothing the connector sends + * is read as a size. + */ + acceptLine: (line: string) => AcceptOutcome; + diagnose: (basis?: readonly string[]) => Diagnosis; + /** + * Section 5.11 gate for one connector hint. `authorized` means the runtime + * may act on the action. Any other result means the hint is shown only as + * the connector's suggestion, and the caller must not act on it. + */ + gateRecoveryHint: (action: string, origin: RecoveryHintOrigin) => HintGateOutcome; + /** + * True when the connector emitted this id earlier in the run, whether the + * fact was accepted or rejected. The ledger is bounded by the fact limit. + */ + hasEmittedId: (id: string) => boolean; + snapshot: () => ObservationSnapshot; +} + +/** Syntax of an OBSERVATION id (schema `observationId`). */ +export function isObservationIdSyntax(value: unknown): value is string { + return validateObservationId(value) === true; +} + +export type ObservationContractErrorCode = + | "descriptor_schema" + | "descriptor_duplicate_id" + | "descriptor_undeclared_step" + | "limits_invalid"; + +/** Bounded error. The message names a code, never the rejected input. */ +export class ObservationContractError extends Error { + readonly code: ObservationContractErrorCode; + + constructor(code: ObservationContractErrorCode) { + super(`observation contract rejected: ${code}`); + this.name = "ObservationContractError"; + this.code = code; + } +} + +// --------------------------------------------------------------------------- +// Constants +// --------------------------------------------------------------------------- + +/** Section 5.10.4 minimums. A runtime must accept at least this much. */ +export const PROFILE_MINIMUM_FACTS = 64; +export const PROFILE_MINIMUM_FACT_BYTES = 2048; + +export const DEFAULT_LIMITS: Required = Object.freeze({ + maxFactBytes: 2048, + maxFacts: 256, + maxPerWindow: 256, + windowMs: 1000, +}); + +const RUNTIME_FACT_TYPES: ReadonlySet = new Set(["credential_submission", "rule_match", "connector_defect"]); + +const CATEGORY_ORDER = ["auth_rejected", "expectation_mismatch"] as const; + +/** The closed Section 5.5 vocabulary. Suggestions are bounded by it: 8 actions times 2 origins. */ +const PORTABLE_RECOVERY_ACTIONS: ReadonlySet = new Set([ + "retry_by_runtime", + "retry_on_connector_upgrade", + "refresh_credentials", + "manual_action_required", + "update_selector", + "upstream_unblock", + "not_retriable", + "unknown", +]); + +// Hints the Section 5.11 gate holds in this version. Every other hint, portable +// or not, is unsupported. +const HINTS_THAT_HOLD: ReadonlySet = new Set(["not_retriable", "unknown"]); + +const HINT_UNSUPPORTED_ID = "runtime:connector_defect:hint_unsupported"; +const PROTOCOL_VIOLATION_ID_PREFIX = "runtime:connector_defect:protocol_violation:"; + +// --------------------------------------------------------------------------- +// Canonical schema, compiled once +// --------------------------------------------------------------------------- + +interface DescriptorsWire { + readonly expectations?: readonly { + readonly id: string; + readonly step: string; + }[]; + readonly rules?: readonly { + readonly id: string; + readonly kind: "auth"; + readonly step?: string; + }[]; + readonly steps?: readonly { readonly id: string }[]; +} + +const ajv = new Ajv2020({ allErrors: false, strict: true }); +ajv.addSchema(observationSchema); + +const SCHEMA_ID = observationSchema.$id; + +// The schema file is a build input. A missing definition is a packaging bug, so it fails at load. +function requireValidator(validator: ValidateFunction | undefined, name: string): ValidateFunction { + if (validator === undefined) { + throw new Error(`observation schema definition is missing: ${name}`); + } + return validator; +} + +const validateMessage = requireValidator(ajv.getSchema(SCHEMA_ID), "observation"); +const validateDescriptorsWire = requireValidator( + ajv.getSchema(`${SCHEMA_ID}#/$defs/diagnosticDescriptors`), + "diagnosticDescriptors" +); +const validateObservationId = requireValidator( + ajv.getSchema(`${SCHEMA_ID}#/$defs/observationId`), + "observationId" +); + +// --------------------------------------------------------------------------- +// Descriptors +// --------------------------------------------------------------------------- + +/** + * Validates the manifest `diagnostic_descriptors` member. An absent member (undefined) + * normalizes to empty arrays. Throws ObservationContractError otherwise. + */ +export function validateDiagnosticDescriptors(value: unknown): DiagnosticDescriptors { + if (value === undefined) { + return { expectations: [], rules: [], steps: [] }; + } + if (!validateDescriptorsWire(value)) { + throw new ObservationContractError("descriptor_schema"); + } + const steps = value.steps ?? []; + const expectations = value.expectations ?? []; + const rules = value.rules ?? []; + + const stepIds = new Set(); + for (const step of steps) { + if (stepIds.has(step.id)) { + throw new ObservationContractError("descriptor_duplicate_id"); + } + stepIds.add(step.id); + } + const expectationIds = new Set(); + for (const expectation of expectations) { + if (expectationIds.has(expectation.id)) { + throw new ObservationContractError("descriptor_duplicate_id"); + } + expectationIds.add(expectation.id); + if (!stepIds.has(expectation.step)) { + throw new ObservationContractError("descriptor_undeclared_step"); + } + } + const ruleIds = new Set(); + for (const rule of rules) { + if (ruleIds.has(rule.id)) { + throw new ObservationContractError("descriptor_duplicate_id"); + } + ruleIds.add(rule.id); + } + + return { + expectations: expectations.map((expectation) => ({ + id: expectation.id, + step: expectation.step, + })), + rules: rules.map((rule) => + rule.step === undefined ? { id: rule.id, kind: rule.kind } : { id: rule.id, kind: rule.kind, step: rule.step } + ), + steps: steps.map((step) => ({ id: step.id })), + }; +} + +// --------------------------------------------------------------------------- +// Limits +// --------------------------------------------------------------------------- + +function resolveLimits(input: ObservationLimits | undefined): Required { + const limits: Required = { + ...DEFAULT_LIMITS, + ...definedOnly(input), + }; + const integers = [limits.maxFacts, limits.maxFactBytes, limits.maxPerWindow, limits.windowMs]; + if (integers.some((n) => !Number.isSafeInteger(n) || n < 1)) { + throw new ObservationContractError("limits_invalid"); + } + if (limits.maxFacts < PROFILE_MINIMUM_FACTS || limits.maxFactBytes < PROFILE_MINIMUM_FACT_BYTES) { + throw new ObservationContractError("limits_invalid"); + } + return limits; +} + +function definedOnly(input: ObservationLimits | undefined): Partial { + const out: { -readonly [K in keyof ObservationLimits]?: number } = {}; + if (input?.maxFacts !== undefined) { + out.maxFacts = input.maxFacts; + } + if (input?.maxFactBytes !== undefined) { + out.maxFactBytes = input.maxFactBytes; + } + if (input?.maxPerWindow !== undefined) { + out.maxPerWindow = input.maxPerWindow; + } + if (input?.windowMs !== undefined) { + out.windowMs = input.windowMs; + } + return out; +} + +// --------------------------------------------------------------------------- +// Run +// --------------------------------------------------------------------------- + +interface StoredFact { + readonly fact: ConnectorFact; + retired: boolean; +} + +interface DeclaredScope { + readonly expectationStep: ReadonlyMap; + readonly rules: ReadonlySet; + readonly steps: ReadonlySet; +} + +/** Creates the state of one run's OBSERVATION intake and diagnosis. */ +export function createObservationRun(input: CreateObservationRunInput): ObservationRun { + const limits = resolveLimits(input.limits); + const now = input.now ?? Date.now; + const scope: DeclaredScope = { + expectationStep: new Map(input.descriptors.expectations.map((expectation) => [expectation.id, expectation.step])), + rules: new Set(input.descriptors.rules.map((rule) => rule.id)), + steps: new Set(input.descriptors.steps.map((step) => step.id)), + }; + + const facts: StoredFact[] = []; + const factsById = new Map(); + const observedIds = new Set(); + const defects = new Map(); + // Keyed by origin and action. The key space is closed (2 origins x 8 + // actions), so the record is bounded without a cap. + const suggestions = new Map(); + let truncated = false; + let windowStart: number | undefined; + let windowCount = 0; + + function recordProtocolViolation(reason: ObservationRejectReason): void { + const id = `${PROTOCOL_VIOLATION_ID_PREFIX}${reason}`; + const previous = defects.get(id); + const count = previous?.class === "protocol_violation" ? previous.count + 1 : 1; + defects.set(id, { class: "protocol_violation", count, id, reason }); + } + + function recordHintUnsupported(): void { + const previous = defects.get(HINT_UNSUPPORTED_ID); + const count = previous?.class === "hint_unsupported" ? previous.count + 1 : 1; + defects.set(HINT_UNSUPPORTED_ID, { + class: "hint_unsupported", + count, + id: HINT_UNSUPPORTED_ID, + }); + } + + function reject(reason: ObservationRejectReason, limitHit: boolean): AcceptOutcome { + recordProtocolViolation(reason); + if (limitHit) { + truncated = true; + } + return { reason, status: "rejected" }; + } + + // Fixed window on the injected clock. A clock that moves backwards starts a new window. + function takeRateSlot(at: number): boolean { + if (windowStart === undefined || at < windowStart || at - windowStart >= limits.windowMs) { + windowStart = at; + windowCount = 0; + } + if (windowCount >= limits.maxPerWindow) { + return false; + } + windowCount += 1; + return true; + } + + function acceptLine(line: string): AcceptOutcome { + if (!input.declared) { + return reject("undeclared_capability", false); + } + const bytes = Buffer.byteLength(line, "utf8"); + let value: unknown; + try { + value = JSON.parse(line); + } catch { + return bytes > limits.maxFactBytes ? reject("oversize", true) : reject("invalid", false); + } + return accept(value, bytes); + } + + function accept(value: unknown, serializedBytes?: number): AcceptOutcome { + if (!input.declared) { + return reject("undeclared_capability", false); + } + const bytes = serializedBytes ?? measureBytes(value); + if (bytes === undefined || !Number.isSafeInteger(bytes) || bytes < 0) { + return reject("invalid", false); + } + // Even a discarded fact has emitted its id (Sections 5.8 and 5.10.2). + const duplicate = rememberEmittedId(value); + if (duplicate) { + return duplicate; + } + if (bytes > limits.maxFactBytes) { + return reject("oversize", true); + } + return admitParsed(value); + } + + function hasEmittedId(id: string): boolean { + return observedIds.has(id); + } + + function rememberEmittedId(value: unknown): AcceptOutcome | null { + // An invalid fact still used its id. Bound this ledger independently + // so a stream of malformed messages cannot consume unbounded memory. + if (typeof value === "object" && value !== null && "id" in value && validateObservationId(value.id)) { + if (observedIds.has(value.id)) { + return reject("duplicate_id", false); + } + if (observedIds.size >= limits.maxFacts) { + return reject("count_exceeded", true); + } + observedIds.add(value.id); + } + return null; + } + + function admitParsed(value: unknown): AcceptOutcome { + if (!takeRateSlot(now())) { + return reject("count_exceeded", true); + } + if (facts.length >= limits.maxFacts) { + return reject("count_exceeded", true); + } + + if (isRuntimeFactType(value)) { + return reject("runtime_fact_type", false); + } + if (!validateMessage(value)) { + return reject("invalid", false); + } + const fact = cloneFrozen(value); + if (fact === undefined) { + return reject("invalid", false); + } + if (fact.fact === "element_expectation" && !fact.attrs.states_seen.includes(fact.attrs.final)) { + return reject("invalid", false); + } + // Retirement applies only to earlier accepted connector facts. Unknown ids are a no-op. + const retired: string[] = []; + for (const id of fact.resolves ?? []) { + const target = factsById.get(id); + if (target !== undefined && !target.retired) { + target.retired = true; + retired.push(id); + } + } + + const stored: StoredFact = { fact, retired: false }; + facts.push(stored); + factsById.set(fact.id, stored); + return { id: fact.id, retired, status: "accepted" }; + } + + function snapshot(): ObservationSnapshot { + const connectorSuggestions = [...suggestions.values()]; + return Object.freeze({ + defects: [...defects.values()].map((defect) => Object.freeze({ ...defect })), + facts: facts.map((stored) => Object.freeze({ ...stored.fact, retired: stored.retired })), + truncated, + ...(connectorSuggestions.length > 0 + ? { + connector_suggestions: Object.freeze(connectorSuggestions.map((s) => Object.freeze({ ...s }))), + } + : {}), + }); + } + + function diagnose(basis: readonly string[] = []): Diagnosis { + const cited = new Set(basis); + const findings: { readonly id: string; readonly category: Category }[] = []; + for (const stored of facts) { + if (stored.retired || !cited.has(stored.fact.id)) { + continue; + } + const category = findingOf(stored.fact, scope); + if (category !== undefined) { + findings.push({ category, id: stored.fact.id }); + } + } + for (const category of CATEGORY_ORDER) { + const supporting = findings.filter((finding) => finding.category === category).map((finding) => finding.id); + if (supporting.length === 0) { + continue; + } + return { + attribution: "connector", + basis: supporting, + category, + contributing: findings.filter((finding) => finding.category !== category).map((finding) => finding.id), + }; + } + return { + attribution: "none", + basis: [], + category: "unknown", + contributing: [], + }; + } + + function recordSuggestion(action: string, origin: RecoveryHintOrigin): void { + if (!isPortableAction(action)) { + return; + } + const key = `${origin}:${action}`; + if (!suggestions.has(key)) { + suggestions.set(key, { action, attribution: "connector", origin }); + } + } + + function gateRecoveryHint(action: string, origin: RecoveryHintOrigin): HintGateOutcome { + if (!HINTS_THAT_HOLD.has(action)) { + recordSuggestion(action, origin); + if (input.declared) { + recordHintUnsupported(); + } + return { result: "unsupported" }; + } + if (truncated) { + recordSuggestion(action, origin); + if (input.declared) { + recordHintUnsupported(); + } + return { reason: "truncated", result: "blocked" }; + } + return { result: "authorized" }; + } + + return { + accept, + acceptLine, + diagnose, + gateRecoveryHint, + hasEmittedId, + snapshot, + }; +} + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +type Category = (typeof CATEGORY_ORDER)[number]; + +/** The diagnosis category a fact supports, or undefined when it supports nothing in this scope. */ +function findingOf(fact: ConnectorFact, scope: DeclaredScope): Category | undefined { + switch (fact.fact) { + case "provider_message": { + if (fact.attrs.kind !== "auth_failure") { + return undefined; + } + if (fact.step !== undefined && !scope.steps.has(fact.step)) { + return undefined; + } + if (fact.attrs.rule !== undefined && !scope.rules.has(fact.attrs.rule)) { + return undefined; + } + return "auth_rejected"; + } + case "element_expectation": { + if (fact.attrs.final === "matched") { + return undefined; + } + if (!scope.steps.has(fact.step)) { + return undefined; + } + return scope.expectationStep.get(fact.attrs.expectation) === fact.step ? "expectation_mismatch" : undefined; + } + case "wait_expired": + return undefined; + } +} + +function isPortableAction(value: string): value is PortableRecoveryAction { + return PORTABLE_RECOVERY_ACTIONS.has(value); +} + +function isRuntimeFactType(value: unknown): boolean { + return typeof value === "object" && value !== null && "fact" in value && RUNTIME_FACT_TYPES.has(value.fact); +} + +function measureBytes(value: unknown): number | undefined { + let json: string | undefined; + try { + json = JSON.stringify(value); + } catch { + return undefined; + } + return typeof json === "string" ? Buffer.byteLength(json, "utf8") : undefined; +} + +/** Copies a validated message so no caller reference is retained, then freezes the copy. */ +function cloneFrozen(value: ConnectorFact): ConnectorFact | undefined { + try { + return deepFreeze(structuredClone(value)); + } catch { + return undefined; + } +} + +function deepFreeze(value: T): T { + if (typeof value === "object" && value !== null) { + for (const child of Object.values(value)) { + deepFreeze(child); + } + Object.freeze(value); + } + return value; +} diff --git a/reference-implementation/scripts/postgres-template-eligibility.ts b/reference-implementation/scripts/postgres-template-eligibility.ts index 3e82b9010..12c9352b4 100644 --- a/reference-implementation/scripts/postgres-template-eligibility.ts +++ b/reference-implementation/scripts/postgres-template-eligibility.ts @@ -27,6 +27,9 @@ * here (or missing from both) fails the inventory test. */ export const POSTGRES_TEMPLATE_ELIGIBLE_FILES: readonly string[] = [ + // Same process-wire and durable diagnostic journeys on seeded SQLite/Postgres, not schema bootstrap. + "test/collection-profile-020.test.ts", + "test/collection-profile-owner-journey.test.ts", "test/accepted-provider-native-consent.test.ts", "test/acknowledged-loss-store-postgres.test.ts", "test/active-run-summary-zero-spine.test.ts", @@ -58,6 +61,8 @@ export const POSTGRES_TEMPLATE_ELIGIBLE_FILES: readonly string[] = [ "test/connector-instance-writer-paths.test.ts", "test/connector-maintenance-cursor-postgres-race.test.ts", "test/connector-manifest-revision-fence.test.ts", + // Uses Postgres as a fixture for the diagnostic run store's writes, retention and locking, not schema bootstrap. + "test/connector-observation-store.test.ts", "test/connector-summary-dirty-hooks.test.ts", "test/connector-summary-evidence-canonical-count-cancel-isolation.test.ts", "test/connector-summary-evidence-direct-write-source-revision-detection.test.ts", diff --git a/reference-implementation/server/backup-table-policy.ts b/reference-implementation/server/backup-table-policy.ts index 39bb7cc12..d09920f36 100644 --- a/reference-implementation/server/backup-table-policy.ts +++ b/reference-implementation/server/backup-table-policy.ts @@ -144,6 +144,11 @@ export const BACKUP_TABLE_INVENTORY: Record = classification: "backup_required", reason: "Maintenance sweep cursors can be retried after crash recovery.", }, + connector_observation_runs: { + classification: "backup_required", + reason: + "Collection Profile diagnostic record per completed run (runtime observation snapshot and diagnosis, bounded to 8 runs per connector). It is owner-visible evidence that cannot be rebuilt from connector state or records, so a restore that drops it loses the diagnosis of recent failed runs.", + }, connector_schedules: { classification: "backup_required", reason: "Collection schedules are durable owner configuration.", diff --git a/reference-implementation/server/db.ts b/reference-implementation/server/db.ts index 7bb43d7bd..090a86b0a 100644 --- a/reference-implementation/server/db.ts +++ b/reference-implementation/server/db.ts @@ -2029,6 +2029,27 @@ CREATE UNIQUE INDEX IF NOT EXISTS idx_connector_coverage_horizons_current CREATE INDEX IF NOT EXISTS idx_connector_coverage_horizons_instance ON connector_coverage_horizons(connector_instance_id); +-- Collection Profile v0.2.0 diagnostic record per completed run (data-connectors +-- spec sections 5.10.3-5.10.4). Holds the runtime's ObservationSnapshot and +-- Diagnosis as JSON and nothing else. server/stores/connector-observation-store.ts +-- keeps at most 8 rows per connector_id, across its instances, and prunes in the +-- write transaction. recorded_seq is the store's per-connector order, never a +-- connector timestamp. Deleting a connection cascades its rows. +CREATE TABLE IF NOT EXISTS connector_observation_runs ( + connector_id TEXT NOT NULL, + connector_instance_id TEXT NOT NULL, + run_id TEXT NOT NULL, + recorded_seq INTEGER NOT NULL, + record_version INTEGER NOT NULL CHECK (record_version = 1), + recorded_at TEXT NOT NULL, + payload_json TEXT NOT NULL, + PRIMARY KEY (connector_id, connector_instance_id, run_id), + FOREIGN KEY(connector_instance_id) REFERENCES connector_instances(connector_instance_id) ON DELETE CASCADE +); + +CREATE UNIQUE INDEX IF NOT EXISTS idx_connector_observation_runs_seq + ON connector_observation_runs(connector_id, recorded_seq); + -- Cross-invocation half of STREAM_EVIDENCE rule 5 ("at most one accepted -- STREAM_EVIDENCE per stream per run_id"): a durable record of every -- (run_id, stream) pair this runtime has ever accepted a STREAM_EVIDENCE diff --git a/reference-implementation/server/index.ts b/reference-implementation/server/index.ts index 13cb81d52..bffb7fe79 100644 --- a/reference-implementation/server/index.ts +++ b/reference-implementation/server/index.ts @@ -510,6 +510,7 @@ import { type ProviderAuthExchanger, } from "./routes/ref-provider-auth.ts"; import { mountRefRecordRejections } from "./routes/ref-record-rejections.ts"; +import { mountRefRunObservations } from "./routes/ref-run-observations.ts"; import { mountRefRunStatus } from "./routes/ref-run-status.ts"; import { mountRefGrants, mountRefRuns, mountRefTraces } from "./routes/ref-spine-correlations.ts"; import { mountRefGrantTimeline, mountRefRunTimeline, mountRefTraceTimeline } from "./routes/ref-spine-timelines.ts"; @@ -5953,6 +5954,12 @@ export function buildAsApp(opts: ServerOpts = {}) { pdppError, requireOwnerSession: ownerAuth.requireOwnerSession, } as unknown as Parameters[1]); + // Owner-only diagnostic read of a run's OBSERVATION record (Collection Profile 0.2.0 Section 5.10). + mountRefRunObservations(app, { + handleError, + pdppError, + requireOwnerSession: ownerAuth.requireOwnerSession, + } as unknown as Parameters[1]); // Run-interaction streaming companion (reference-only). The store and // companion factory live in this AS app so the mint route, the SSE viewer diff --git a/reference-implementation/server/postgres-storage.ts b/reference-implementation/server/postgres-storage.ts index 05dbe3c00..1001e0e9e 100644 --- a/reference-implementation/server/postgres-storage.ts +++ b/reference-implementation/server/postgres-storage.ts @@ -2799,6 +2799,23 @@ async function bootstrapPostgresSchemaOnce({ CREATE INDEX IF NOT EXISTS idx_pg_connector_coverage_horizons_instance ON connector_coverage_horizons(connector_instance_id); + -- Collection Profile v0.2.0 diagnostic record per completed run. See the + -- matching SQLite DDL comment in server/db.ts (connector_observation_runs). + -- The store keeps 8 rows per connector_id and prunes in its write transaction. + CREATE TABLE IF NOT EXISTS connector_observation_runs ( + connector_id TEXT NOT NULL, + connector_instance_id TEXT NOT NULL REFERENCES connector_instances(connector_instance_id) ON DELETE CASCADE, + run_id TEXT NOT NULL, + recorded_seq BIGINT NOT NULL, + record_version INTEGER NOT NULL CHECK (record_version = 1), + recorded_at TEXT NOT NULL, + payload_json TEXT NOT NULL, + PRIMARY KEY (connector_id, connector_instance_id, run_id) + ); + + CREATE UNIQUE INDEX IF NOT EXISTS idx_pg_connector_observation_runs_seq + ON connector_observation_runs(connector_id, recorded_seq); + -- Cross-invocation STREAM_EVIDENCE duplicate registry — see the -- matching SQLite DDL comment in server/db.ts -- (stream_evidence_run_registry) for the full rationale. Primary key diff --git a/reference-implementation/server/routes/ref-run-observations.ts b/reference-implementation/server/routes/ref-run-observations.ts new file mode 100644 index 000000000..b35cb2eeb --- /dev/null +++ b/reference-implementation/server/routes/ref-run-observations.ts @@ -0,0 +1,79 @@ +// Copyright The PDP-Connect Contributors +// SPDX-License-Identifier: Apache-2.0 + +// Owner-only diagnostic read of one run's OBSERVATION record (Collection +// Profile 0.2.0 Sections 5.10 and 5.11). +// +// GET /_ref/runs/:runId/observations?connector_id=...&connector_instance_id=... +// +// The record is keyed by connector, connector instance and run. A run id that +// exists under another connector or instance is absent, so it returns a typed +// `not_found` 404. This route exports nothing: it returns the stored record to +// the owner session that asked, and nothing else reads it. + +import type { ConnectorObservationRunStore } from "../stores/connector-observation-store.ts"; +import { getDefaultConnectorObservationStore } from "../stores/connector-observation-store.ts"; +import type { MiddlewareHandler, PdppErrorFn } from "./_route-contract.ts"; + +interface RouteRequest { + readonly params: Readonly>; + readonly query?: Readonly>; +} + +interface RouteResponse { + json: (body: unknown) => unknown; + setHeader: (name: string, value: string) => unknown; +} + +type RouteHandler = (req: RouteRequest, res: RouteResponse) => unknown | Promise; + +interface AppLike { + get: (path: string, ...handlers: (MiddlewareHandler | RouteHandler)[]) => AppLike; +} + +export interface MountRefRunObservationsContext { + handleError: (res: unknown, err: unknown) => void; + pdppError: PdppErrorFn; + requireOwnerSession: MiddlewareHandler; + /** Injectable for tests. Defaults to the process store for the active backend. */ + store?: ConnectorObservationRunStore; +} + +function requiredQueryString(query: RouteRequest["query"], key: string): string | null { + const value = query?.[key]; + return typeof value === "string" && value.length > 0 ? value : null; +} + +export function mountRefRunObservations(app: AppLike, ctx: MountRefRunObservationsContext): void { + app.get("/_ref/runs/:runId/observations", ctx.requireOwnerSession, async (req: RouteRequest, res: RouteResponse) => { + res.setHeader("Cache-Control", "no-store"); + try { + const runId = decodeURIComponent(req.params.runId as string); + const connectorId = requiredQueryString(req.query, "connector_id"); + const connectorInstanceId = requiredQueryString(req.query, "connector_instance_id"); + if (connectorId === null) { + return ctx.pdppError(res, 400, "invalid_request", "connector_id is required", "connector_id"); + } + if (connectorInstanceId === null) { + return ctx.pdppError(res, 400, "invalid_request", "connector_instance_id is required", "connector_instance_id"); + } + + const store = ctx.store ?? getDefaultConnectorObservationStore(); + const record = await store.get({ connectorId, connectorInstanceId, runId }); + if (record === null) { + return ctx.pdppError(res, 404, "not_found", `Run observations not found: ${runId}`, "run_id"); + } + return res.json({ + connector_id: record.connectorId, + connector_instance_id: record.connectorInstanceId, + diagnosis: record.diagnosis, + object: "run_observations", + recorded_at: record.recordedAt, + run_id: record.runId, + snapshot: record.snapshot, + }); + } catch (err) { + return ctx.handleError(res, err); + } + }); +} diff --git a/reference-implementation/server/stores/connector-observation-store.ts b/reference-implementation/server/stores/connector-observation-store.ts new file mode 100644 index 000000000..06df970ac --- /dev/null +++ b/reference-implementation/server/stores/connector-observation-store.ts @@ -0,0 +1,448 @@ +// Copyright The PDP-Connect Contributors +// SPDX-License-Identifier: Apache-2.0 + +/** + * Diagnostic records for completed connector runs (Collection Profile v0.2.0, + * data-connectors docs spec sections 5.10.3 and 5.10.4). + * + * A record holds one run's ObservationSnapshot and Diagnosis, the typed values + * `runtime/observations.ts` produces, with the run and connection identity. + * The store writes no other field: no raw connector line, provider prose, + * free-form DONE text or credential. Retirement flags and runtime defects are + * stored as the typed runtime supplied them. + * + * Bounds: + * - At most CONNECTOR_OBSERVATION_RUNS_PER_CONNECTOR records per connector_id, + * counted across all of its connector_instance_id values. Surplus records are + * pruned in the same transaction as the write that creates them. + * - A record's serialized payload is at most CONNECTOR_OBSERVATION_RECORD_MAX_BYTES. + * A larger record is rejected before any database access. Total payload per + * connector is therefore at most 8 MiB plus fixed row overhead. + * + * Ordering is `recorded_seq`, a per-connector sequence this store assigns inside + * the write transaction. Connector-provided timestamps are never used. + * + * Concurrency: SQLite writes run under BEGIN IMMEDIATE. PostgreSQL writes take a + * transaction-scoped advisory lock keyed by connector_id, which serializes + * writers in every process that shares the database. + */ + +import { execDynamicSqlAcknowledged, iterateDynamicSqlAcknowledged, writeTransaction } from "../../lib/db.ts"; +import { RECOVERY_ACTIONS } from "../../runtime/connector-gap-bounding.ts"; +import type { Diagnosis, ObservationSnapshot } from "../../runtime/observations.ts"; +import { + getStorageBackendKind, + isPostgresStorageBackend, + postgresQuery, + withPostgresTransaction, +} from "../postgres-storage.ts"; + +export const CONNECTOR_OBSERVATION_RUNS_PER_CONNECTOR = 8; +export const CONNECTOR_OBSERVATION_RECORD_MAX_BYTES = 1024 * 1024; + +const RECORD_VERSION = 1; +const MAX_IDENTITY_LENGTH = 256; +const DIAGNOSIS_CATEGORIES: ReadonlySet = new Set(["auth_rejected", "expectation_mismatch", "unknown"]); +const DIAGNOSIS_ATTRIBUTIONS: ReadonlySet = new Set(["connector", "none"]); +const LOCK_NAMESPACE = "connector_observation_runs:"; + +export type ConnectorObservationStoreErrorCode = "record_invalid" | "record_oversize" | "record_conflict"; + +/** Bounded error. The message names a rule, never the rejected record's content. */ +export class ConnectorObservationStoreError extends Error { + readonly code: ConnectorObservationStoreErrorCode; + + constructor(code: ConnectorObservationStoreErrorCode, message: string) { + super(message); + this.name = "ConnectorObservationStoreError"; + this.code = code; + } +} + +export interface ConnectorObservationRunKey { + readonly connectorId: string; + readonly connectorInstanceId: string; + readonly runId: string; +} + +export interface ConnectorObservationRunInput extends ConnectorObservationRunKey { + readonly diagnosis: Diagnosis; + readonly snapshot: ObservationSnapshot; +} + +export interface ConnectorObservationRunRecord extends ConnectorObservationRunInput { + /** Runtime clock at write time. Ordering uses the store's recorded sequence, not this value. */ + readonly recordedAt: string; +} + +export interface ListConnectorObservationRunsInput { + readonly connectorId: string; + /** Omitted: every instance of the connector. */ + readonly connectorInstanceId?: string; + /** Clamped to CONNECTOR_OBSERVATION_RUNS_PER_CONNECTOR. */ + readonly limit?: number; +} + +export interface ConnectorObservationRunStore { + get: (key: ConnectorObservationRunKey) => Promise; + /** Newest first, by recorded sequence. */ + list: (input: ListConnectorObservationRunsInput) => Promise; + /** + * Writes one completed run. A repeat of the same key with the same payload + * is a no-op. The same key with a different payload rejects with + * `record_conflict`. + */ + put: (record: ConnectorObservationRunInput) => Promise; +} + +interface RunRow { + connector_id: string; + connector_instance_id: string; + payload_json: string; + recorded_at: string; + run_id: string; +} + +interface PayloadRow { + payload_json: string; +} + +interface NextSequenceRow { + next_seq: number | string; +} + +// Static SQL per backend. Both statements bind every value; the only literal +// is the fixed record version, which is a column constant. +const SQLITE_SQL = { + existing: + "SELECT payload_json FROM connector_observation_runs WHERE connector_id = ? AND connector_instance_id = ? AND run_id = ?", + get: "SELECT connector_id, connector_instance_id, run_id, recorded_at, payload_json FROM connector_observation_runs WHERE connector_id = ? AND connector_instance_id = ? AND run_id = ?", + insert: + "INSERT INTO connector_observation_runs (connector_id, connector_instance_id, run_id, recorded_seq, record_version, recorded_at, payload_json) VALUES (?, ?, ?, ?, ?, ?, ?)", + listAllInstances: + "SELECT connector_id, connector_instance_id, run_id, recorded_at, payload_json FROM connector_observation_runs WHERE connector_id = ? ORDER BY recorded_seq DESC LIMIT ?", + listOneInstance: + "SELECT connector_id, connector_instance_id, run_id, recorded_at, payload_json FROM connector_observation_runs WHERE connector_id = ? AND connector_instance_id = ? ORDER BY recorded_seq DESC LIMIT ?", + nextSequence: + "SELECT COALESCE(MAX(recorded_seq), 0) + 1 AS next_seq FROM connector_observation_runs WHERE connector_id = ?", + prune: + "DELETE FROM connector_observation_runs WHERE connector_id = ? AND recorded_seq NOT IN (SELECT recorded_seq FROM connector_observation_runs WHERE connector_id = ? ORDER BY recorded_seq DESC LIMIT ?)", +} as const; + +const POSTGRES_SQL = { + existing: + "SELECT payload_json FROM connector_observation_runs WHERE connector_id = $1 AND connector_instance_id = $2 AND run_id = $3", + get: "SELECT connector_id, connector_instance_id, run_id, recorded_at, payload_json FROM connector_observation_runs WHERE connector_id = $1 AND connector_instance_id = $2 AND run_id = $3", + insert: + "INSERT INTO connector_observation_runs (connector_id, connector_instance_id, run_id, recorded_seq, record_version, recorded_at, payload_json) VALUES ($1, $2, $3, $4, $5, $6, $7)", + listAllInstances: + "SELECT connector_id, connector_instance_id, run_id, recorded_at, payload_json FROM connector_observation_runs WHERE connector_id = $1 ORDER BY recorded_seq DESC LIMIT $2", + listOneInstance: + "SELECT connector_id, connector_instance_id, run_id, recorded_at, payload_json FROM connector_observation_runs WHERE connector_id = $1 AND connector_instance_id = $2 ORDER BY recorded_seq DESC LIMIT $3", + nextSequence: + "SELECT COALESCE(MAX(recorded_seq), 0) + 1 AS next_seq FROM connector_observation_runs WHERE connector_id = $1", + prune: + "DELETE FROM connector_observation_runs WHERE connector_id = $1 AND recorded_seq NOT IN (SELECT recorded_seq FROM connector_observation_runs WHERE connector_id = $2 ORDER BY recorded_seq DESC LIMIT $3)", +} as const; + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function isStringArray(value: unknown): value is string[] { + return Array.isArray(value) && value.every((item) => typeof item === "string"); +} + +function isSnapshot(value: unknown): value is ObservationSnapshot { + return ( + isRecord(value) && + Array.isArray(value.facts) && + Array.isArray(value.defects) && + typeof value.truncated === "boolean" && + (value.connector_suggestions === undefined || isConnectorSuggestions(value.connector_suggestions)) + ); +} + +function isConnectorSuggestions(value: unknown): boolean { + return ( + Array.isArray(value) && + value.length <= 16 && + value.every( + (suggestion: unknown) => + isRecord(suggestion) && + Object.keys(suggestion).every((key) => key === "attribution" || key === "action" || key === "origin") && + suggestion.attribution === "connector" && + typeof suggestion.action === "string" && + RECOVERY_ACTIONS.has(suggestion.action) && + (suggestion.origin === "skip_result" || suggestion.origin === "done_error") + ) + ); +} + +function isDiagnosis(value: unknown): value is Diagnosis { + return ( + isRecord(value) && + typeof value.category === "string" && + DIAGNOSIS_CATEGORIES.has(value.category) && + typeof value.attribution === "string" && + DIAGNOSIS_ATTRIBUTIONS.has(value.attribution) && + isStringArray(value.basis) && + isStringArray(value.contributing) + ); +} + +function invalid(message: string): ConnectorObservationStoreError { + return new ConnectorObservationStoreError("record_invalid", message); +} + +function requireIdentity(field: string, value: unknown): string { + if (typeof value !== "string" || value.trim().length === 0 || value.length > MAX_IDENTITY_LENGTH) { + throw invalid( + `connector observation ${field} must be a non-blank string of at most ${MAX_IDENTITY_LENGTH} characters` + ); + } + return value; +} + +function requireKey(key: ConnectorObservationRunKey): ConnectorObservationRunKey { + return { + connectorId: requireIdentity("connectorId", key.connectorId), + connectorInstanceId: requireIdentity("connectorInstanceId", key.connectorInstanceId), + runId: requireIdentity("runId", key.runId), + }; +} + +function boundedLimit(limit: number | undefined): number { + if (limit === undefined) { + return CONNECTOR_OBSERVATION_RUNS_PER_CONNECTOR; + } + if (!Number.isSafeInteger(limit) || limit < 1) { + throw invalid("connector observation limit must be a positive integer"); + } + return Math.min(limit, CONNECTOR_OBSERVATION_RUNS_PER_CONNECTOR); +} + +/** + * Validates a record and serializes exactly the persisted fields. Every check + * runs here, before any database access, so a rejected record mutates nothing. + */ +function encodeRecord(input: ConnectorObservationRunInput): { + key: ConnectorObservationRunKey; + payload: string; +} { + const key = requireKey(input); + if (!(isSnapshot(input.snapshot) && isDiagnosis(input.diagnosis))) { + throw invalid("connector observation snapshot and diagnosis must match the runtime shapes"); + } + const { facts, defects, truncated, connector_suggestions } = input.snapshot; + const { category, attribution, basis, contributing } = input.diagnosis; + const payload = JSON.stringify({ + diagnosis: { attribution, basis, category, contributing }, + snapshot: { defects, facts, truncated, ...(connector_suggestions === undefined ? {} : { connector_suggestions }) }, + }); + if (Buffer.byteLength(payload, "utf8") > CONNECTOR_OBSERVATION_RECORD_MAX_BYTES) { + throw new ConnectorObservationStoreError( + "record_oversize", + `connector observation record exceeds ${CONNECTOR_OBSERVATION_RECORD_MAX_BYTES} bytes` + ); + } + return { key, payload }; +} + +function decodeRow(row: RunRow): ConnectorObservationRunRecord { + const parsed: unknown = JSON.parse(row.payload_json); + if (!(isRecord(parsed) && isSnapshot(parsed.snapshot) && isDiagnosis(parsed.diagnosis))) { + throw invalid("stored connector observation record is malformed"); + } + return { + connectorId: row.connector_id, + connectorInstanceId: row.connector_instance_id, + diagnosis: parsed.diagnosis, + recordedAt: row.recorded_at, + runId: row.run_id, + snapshot: parsed.snapshot, + }; +} + +function assertSamePayload(stored: string, incoming: string): void { + if (stored !== incoming) { + throw new ConnectorObservationStoreError( + "record_conflict", + "connector observation run is already recorded with different content" + ); + } +} + +/** + * The SQLite store runs synchronously. A throw still reaches the caller as a + * rejected promise, the same contract the Postgres methods have. + */ +function settle(step: () => T): Promise { + try { + return Promise.resolve(step()); + } catch (error) { + return Promise.reject(error); + } +} + +function createSqliteConnectorObservationRunStore(): ConnectorObservationRunStore { + return { + get(input: ConnectorObservationRunKey): Promise { + return settle(() => { + const key = requireKey(input); + const [row] = [ + ...iterateDynamicSqlAcknowledged(SQLITE_SQL.get, [ + key.connectorId, + key.connectorInstanceId, + key.runId, + ]), + ]; + return row ? decodeRow(row) : null; + }); + }, + list(input: ListConnectorObservationRunsInput): Promise { + return settle(() => { + const connectorId = requireIdentity("connectorId", input.connectorId); + const limit = boundedLimit(input.limit); + const rows = + input.connectorInstanceId === undefined + ? [...iterateDynamicSqlAcknowledged(SQLITE_SQL.listAllInstances, [connectorId, limit])] + : [ + ...iterateDynamicSqlAcknowledged(SQLITE_SQL.listOneInstance, [ + connectorId, + requireIdentity("connectorInstanceId", input.connectorInstanceId), + limit, + ]), + ]; + return rows.map(decodeRow); + }); + }, + put(input: ConnectorObservationRunInput): Promise { + return settle(() => { + const { key, payload } = encodeRecord(input); + const recordedAt = new Date().toISOString(); + writeTransaction(() => { + const [existing] = [ + ...iterateDynamicSqlAcknowledged(SQLITE_SQL.existing, [ + key.connectorId, + key.connectorInstanceId, + key.runId, + ]), + ]; + if (existing) { + assertSamePayload(existing.payload_json, payload); + return; + } + const [sequence] = [ + ...iterateDynamicSqlAcknowledged(SQLITE_SQL.nextSequence, [key.connectorId]), + ]; + // REVIEWED-DYNAMIC: fixed static statements; every value is bound. + execDynamicSqlAcknowledged(SQLITE_SQL.insert, [ + key.connectorId, + key.connectorInstanceId, + key.runId, + Number(sequence?.next_seq ?? 1), + RECORD_VERSION, + recordedAt, + payload, + ]); + // REVIEWED-DYNAMIC: fixed static statement; retention is per connector_id across instances. + execDynamicSqlAcknowledged(SQLITE_SQL.prune, [ + key.connectorId, + key.connectorId, + CONNECTOR_OBSERVATION_RUNS_PER_CONNECTOR, + ]); + }); + }); + }, + }; +} + +function createPostgresConnectorObservationRunStore(): ConnectorObservationRunStore { + return { + async get(input: ConnectorObservationRunKey): Promise { + const key = requireKey(input); + const result = await postgresQuery(POSTGRES_SQL.get, [ + key.connectorId, + key.connectorInstanceId, + key.runId, + ]); + const [row] = result.rows; + return row ? decodeRow(row) : null; + }, + async list(input: ListConnectorObservationRunsInput): Promise { + const connectorId = requireIdentity("connectorId", input.connectorId); + const limit = boundedLimit(input.limit); + const result = + input.connectorInstanceId === undefined + ? await postgresQuery(POSTGRES_SQL.listAllInstances, [connectorId, limit]) + : await postgresQuery(POSTGRES_SQL.listOneInstance, [ + connectorId, + requireIdentity("connectorInstanceId", input.connectorInstanceId), + limit, + ]); + return result.rows.map(decodeRow); + }, + async put(input: ConnectorObservationRunInput): Promise { + const { key, payload } = encodeRecord(input); + const recordedAt = new Date().toISOString(); + await withPostgresTransaction(async (client) => { + // Transaction-scoped, so it releases at COMMIT or ROLLBACK, and it + // serializes writers in every process that shares this database. + await client.query("SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", [ + `${LOCK_NAMESPACE}${key.connectorId}`, + ]); + const existing = await client.query(POSTGRES_SQL.existing, [ + key.connectorId, + key.connectorInstanceId, + key.runId, + ]); + const [current] = existing.rows; + if (current) { + assertSamePayload(current.payload_json, payload); + return; + } + const sequence = await client.query(POSTGRES_SQL.nextSequence, [key.connectorId]); + await client.query(POSTGRES_SQL.insert, [ + key.connectorId, + key.connectorInstanceId, + key.runId, + Number(sequence.rows[0]?.next_seq ?? 1), + RECORD_VERSION, + recordedAt, + payload, + ]); + await client.query(POSTGRES_SQL.prune, [ + key.connectorId, + key.connectorId, + CONNECTOR_OBSERVATION_RUNS_PER_CONNECTOR, + ]); + }); + }, + }; +} + +export function createConnectorObservationRunStore(): ConnectorObservationRunStore { + return isPostgresStorageBackend() + ? createPostgresConnectorObservationRunStore() + : createSqliteConnectorObservationRunStore(); +} + +let defaultStore: ConnectorObservationRunStore | null = null; +let defaultBackend: string | null = null; + +/** Cached singleton keyed by storage backend, mirroring `getDefaultConnectorCoverageHorizonStore`. */ +export function getDefaultConnectorObservationStore(): ConnectorObservationRunStore { + const backend = getStorageBackendKind(); + if (!defaultStore || defaultBackend !== backend) { + defaultStore = createConnectorObservationRunStore(); + defaultBackend = backend; + } + return defaultStore; +} + +/** Test and reinitialization seam: drops the cached singleton so the next call rebinds to the active backend. */ +export function resetDefaultConnectorObservationStoreCache(): void { + defaultStore = null; + defaultBackend = null; +} diff --git a/reference-implementation/test/collection-profile-020.test.ts b/reference-implementation/test/collection-profile-020.test.ts new file mode 100644 index 000000000..9b1b88083 --- /dev/null +++ b/reference-implementation/test/collection-profile-020.test.ts @@ -0,0 +1,615 @@ +// Copyright The PDP-Connect Contributors +// SPDX-License-Identifier: Apache-2.0 + +// Collection Profile 0.2.0 runtime journeys. Each test runs a real connector +// child process through runConnector and asserts on the observable outcome: +// the run result, the persisted diagnostic record, the trace file, or whether +// the child executed at all. Connector stubs are generated files, so the +// runtime's own parsing, validation, and gates are what the tests exercise. +// +// Spec: data-connectors docs/spec/collection-profile.md, Sections 3.4, 3.8, +// 5.5, 5.8, 5.10 to 5.13, 6.2 and 8. + +import assert from "node:assert/strict"; +import { mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import test, { after, before } from "node:test"; +import { runConnector } from "../runtime/index.ts"; +import { closeDb, getDb, initDb } from "../server/db.ts"; +import { closePostgresStorage, initPostgresStorage, postgresQuery } from "../server/postgres-storage.ts"; +import { createRequestConnectorInstanceStore } from "../server/request-store-factories.ts"; +import { admitOwnerRunConnection } from "../server/stores/connector-instance-store.ts"; +import { getDefaultConnectorObservationStore } from "../server/stores/connector-observation-store.ts"; +import { dedicatedPostgresTestUrl } from "./helpers/dedicated-postgres-test-url.ts"; + +const CONNECTOR_ID = "collection-profile-020-test"; +const SECRET_MARKER = "SECRET-MARKER-7f3a"; +const INVALID_DONE_STATUS = /invalid DONE status/; +const PROTOCOL_VERSION_NAMED = /protocol_version/; +const OBSERVATION_NEEDS_020 = /OBSERVATION require protocol_version 0\.2\.0/; +const DIAGNOSTIC_DESCRIPTORS_NEEDS_020 = /diagnostic_descriptors require protocol_version 0\.2\.0/; +const UNDECLARED_STEP = /descriptor_undeclared_step/; +const RETRYABLE_FIELD = /DONE\.error\.retryable/; +// Backend under test. With the dedicated private PostgreSQL URL the runtime, the +// admission store and the observation store all use Postgres. Without it the +// file runs on in-memory SQLite, as before. The journey and assertions are the +// same on both backends. No shared or production database is ever selected. +const POSTGRES_URL = dedicatedPostgresTestUrl(process.env.PDPP_TEST_POSTGRES_URL); +// The admitted instance row for this connector. Set in `before`, so each run +// and each persisted-record lookup use a real instance, as production does. +let INSTANCE_ID = ""; + +before(async () => { + if (POSTGRES_URL) { + await initPostgresStorage({ backend: "postgres", databaseUrl: POSTGRES_URL }); + // The connector_instances row references connectors(connector_id); admission + // fails its foreign key check for an unregistered connector. + await postgresQuery( + "INSERT INTO connectors(connector_id, manifest) VALUES ($1, $2::jsonb) ON CONFLICT DO NOTHING", + [CONNECTOR_ID, JSON.stringify({ connector_id: CONNECTOR_ID })] + ); + } else { + initDb(":memory:"); + // The connector_instances row references connectors(connector_id); admission + // fails its foreign key check for an unregistered connector. + getDb() + .prepare("INSERT OR IGNORE INTO connectors(connector_id, manifest, created_at) VALUES (?, ?, ?)") + .run(CONNECTOR_ID, JSON.stringify({ connector_id: CONNECTOR_ID }), "2026-11-02T00:00:00.000Z"); + } + const namespace = await admitOwnerRunConnection({ + connectorId: CONNECTOR_ID, + connectorInstanceStore: createRequestConnectorInstanceStore(), + ownerSubjectId: "owner_local", + }); + INSTANCE_ID = namespace.connectorInstanceId; +}); + +after(async () => { + if (POSTGRES_URL) { + await closePostgresStorage(); + } else { + closeDb(); + } +}); + +interface Stub { + readonly connectorPath: string; + readonly dir: string; + readonly markerPath: string; +} + +// The child writes a marker file as soon as it receives START. A test that +// expects no spawn checks that the marker is absent. +function makeStub(messages: readonly unknown[], exitCode: number): Stub { + const dir = mkdtempSync(join(tmpdir(), "pdpp-collection-profile-020-")); + const connectorPath = join(dir, "connector.mjs"); + const markerPath = join(dir, "spawned.marker"); + writeFileSync( + connectorPath, + ` +import { createInterface } from 'node:readline'; +import { writeFileSync } from 'node:fs'; +const rl = createInterface({ input: process.stdin, terminal: false }); +rl.on('line', (line) => { + if (JSON.parse(line).type !== 'START') return; + writeFileSync(${JSON.stringify(markerPath)}, line); + for (const message of ${JSON.stringify(messages)}) process.stdout.write(JSON.stringify(message) + '\\n'); + rl.close(); + process.exit(${exitCode}); +}); +`, + "utf8" + ); + return { connectorPath, dir, markerPath }; +} + +const DECLARED_DESCRIPTORS = { + expectations: [{ id: "email_input", step: "sign_in" }], + rules: [{ id: "password_rejected", kind: "auth", step: "sign_in" }], + steps: [{ id: "sign_in" }], +}; + +function manifest(overrides: Record = {}) { + return { + connector_id: CONNECTOR_ID, + diagnostic_descriptors: DECLARED_DESCRIPTORS, + protocol_capabilities: ["OBSERVATION"], + protocol_version: "0.2.0", + streams: [{ name: "items" }], + ...overrides, + }; +} + +type RunOutcome = + | { readonly ok: true; readonly result: Record } + | { + readonly ok: false; + readonly message: string; + readonly connectorError: { readonly message?: string } | null; + }; + +// A runtime rejection becomes a value, so a failing assertion names the +// outcome instead of crashing the test file. +async function runStub( + stub: Stub, + runId: string, + manifestValue: Record, + extraEnv: Record = {} +): Promise { + const savedEnv: Record = {}; + for (const [key, value] of Object.entries(extraEnv)) { + savedEnv[key] = process.env[key]; + process.env[key] = value; + } + try { + const result = await runConnector({ + admitRunConnection: async ({ + connectorId, + connectorInstanceId, + ownerSubjectId, + }: { + connectorId: string; + connectorInstanceId?: string; + ownerSubjectId?: string; + }) => { + const namespace = await admitOwnerRunConnection({ + connectorId, + connectorInstanceId: connectorInstanceId ?? INSTANCE_ID, + connectorInstanceStore: createRequestConnectorInstanceStore(), + ownerSubjectId: ownerSubjectId ?? "owner_local", + }); + return { + connectorId: namespace.connectorId, + connectorInstanceId: namespace.connectorInstanceId, + ownerSubjectId: ownerSubjectId ?? "owner_local", + }; + }, + connectorId: CONNECTOR_ID, + connectorPath: stub.connectorPath, + manifest: manifestValue as never, + onInteraction: async () => ({}), + onProgress: () => { + // Progress events are not asserted by these tests. + }, + ownerToken: "test-owner-token", + persistState: false, + runId, + scope: { streams: [{ name: "items" }] }, + } as never); + return { ok: true, result: result as unknown as Record }; + } catch (caught) { + const connectorError = (caught as { connector_error?: { message?: string } | null }).connector_error ?? null; + return { connectorError, message: (caught as Error).message, ok: false }; + } finally { + for (const [key, value] of Object.entries(savedEnv)) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + } +} + +function persistedRecord(runId: string) { + return getDefaultConnectorObservationStore().get({ + connectorId: CONNECTOR_ID, + connectorInstanceId: INSTANCE_ID, + runId, + }); +} + +// A protocol violation is keyed by its reason; a hint defect by its class. +function defectKey(defect: { class: string; reason?: string }): string { + return defect.class === "protocol_violation" ? (defect.reason ?? "") : defect.class; +} + +function observation(id: string, fields: Record) { + return { id, type: "OBSERVATION", ...fields }; +} + +const EXPECTATION = (id: string) => + observation(id, { + attrs: { expectation: "email_input", final: "absent", states_seen: ["absent"] }, + fact: "element_expectation", + step: "sign_in", + }); + +const PROVIDER_REJECTED = (id: string, rule = "password_rejected") => + observation(id, { + attrs: { kind: "auth_failure", rule }, + fact: "provider_message", + step: "sign_in", + }); + +test("declared 0.2.0 OBSERVATION facts are accepted and the run succeeds", async () => { + const stub = makeStub([EXPECTATION("o1"), { records_emitted: 0, status: "succeeded", type: "DONE" }], 0); + try { + const outcome = await runStub(stub, "run_c020_accept", manifest()); + assert.equal(outcome.ok, true, `declared OBSERVATION must not abort the run: ${outcome.ok ? "" : outcome.message}`); + if (!outcome.ok) { + return; + } + assert.equal(outcome.result.status, "succeeded"); + const start = JSON.parse(readFileSync(stub.markerPath, "utf8")) as { protocol_capabilities: string[] }; + assert.ok(start.protocol_capabilities.includes("OBSERVATION"), "START advertises the implemented message"); + const record = await persistedRecord("run_c020_accept"); + assert.ok(record, "the run's bounded diagnostic record must be persisted"); + assert.deepEqual( + record.snapshot.facts.map((fact) => fact.id), + ["o1"] + ); + assert.deepEqual(record.snapshot.defects, []); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } +}); + +test("a portable 0.2.0 connector cannot report runtime cancellation as a DONE status", async () => { + const stub = makeStub([{ records_emitted: 0, status: "cancelled", type: "DONE" }], 1); + try { + const outcome = await runStub(stub, "run_c020_cancelled_status", manifest()); + assert.equal(outcome.ok, false, "Section 5.8 permits only succeeded and failed connector status values"); + if (!outcome.ok) { + assert.match(outcome.message, INVALID_DONE_STATUS); + } + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } +}); + +test("an oversized observation still owns its emitted id and can be cited without corrupting collection", async () => { + const stub = makeStub( + [ + { ...EXPECTATION("large_id"), padding: "x".repeat(3000) }, + EXPECTATION("large_id"), + { + error: { basis: ["large_id"], message: "Connector reported failure", retryable: false }, + records_emitted: 0, + status: "failed", + type: "DONE", + }, + ], + 1 + ); + try { + const outcome = await runStub(stub, "run_c020_oversize_id", manifest()); + assert.equal( + outcome.ok, + true, + "discarding an oversized fact must not turn a valid DONE citation into a protocol failure" + ); + const record = await persistedRecord("run_c020_oversize_id"); + assert.ok(record); + assert.deepEqual(record.snapshot.facts, [], "a later fact must not reuse an id from the oversized message"); + assert.deepEqual(record.snapshot.defects.map(defectKey).sort(), ["duplicate_id", "oversize"]); + assert.equal(record.snapshot.truncated, true); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } +}); + +test("an undeclared OBSERVATION is discarded as a protocol violation and collection still succeeds", async () => { + const stub = makeStub([EXPECTATION("u1"), { records_emitted: 0, status: "succeeded", type: "DONE" }], 0); + try { + const outcome = await runStub( + stub, + "run_c020_undeclared", + manifest({ diagnostic_descriptors: undefined, protocol_capabilities: undefined, protocol_version: "0.1.0" }) + ); + assert.equal( + outcome.ok, + true, + `an undeclared OBSERVATION must not abort the run: ${outcome.ok ? "" : outcome.message}` + ); + if (!outcome.ok) { + return; + } + assert.equal(outcome.result.status, "succeeded"); + const record = await persistedRecord("run_c020_undeclared"); + assert.ok(record, "a legacy run's diagnostic record must still be persisted"); + assert.deepEqual(record.snapshot.facts, []); + assert.deepEqual(record.snapshot.defects.map(defectKey), ["undeclared_capability"]); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } +}); + +test("malformed, runtime-typed, duplicate and oversize OBSERVATIONs are discarded and later collection succeeds", async () => { + const stub = makeStub( + [ + observation("bad1", { + attrs: { expectation: "email_input", final: "absent", secret: SECRET_MARKER, states_seen: ["absent"] }, + fact: "element_expectation", + step: "sign_in", + }), + observation("rt1", { attrs: { handle: "h1", kind: "auth", rule: "password_rejected" }, fact: "rule_match" }), + EXPECTATION("dup1"), + EXPECTATION("dup1"), + observation("big1", { + attrs: { awaited: "email_input", budget_ms: 10 }, + fact: "wait_expired", + padding: "x".repeat(3000), + step: "sign_in", + }), + EXPECTATION("ok2"), + { records_emitted: 0, status: "succeeded", type: "DONE" }, + ], + 0 + ); + try { + const outcome = await runStub(stub, "run_c020_discard", manifest()); + assert.equal( + outcome.ok, + true, + `discarded OBSERVATIONs must not abort the run: ${outcome.ok ? "" : outcome.message}` + ); + if (!outcome.ok) { + return; + } + assert.equal(outcome.result.status, "succeeded"); + const record = await persistedRecord("run_c020_discard"); + assert.ok(record, "the diagnostic record must be persisted"); + assert.deepEqual( + record.snapshot.facts.map((fact) => fact.id), + ["dup1", "ok2"] + ); + const reasons = record.snapshot.defects.map(defectKey); + assert.deepEqual(reasons.sort(), ["duplicate_id", "invalid", "oversize", "runtime_fact_type"]); + assert.equal(JSON.stringify(record).includes(SECRET_MARKER), false, "rejected attrs must not reach the store"); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } +}); + +test("raw OBSERVATION lines, including rejected ones, never reach the runtime trace", async () => { + const traceDir = mkdtempSync(join(tmpdir(), "pdpp-collection-profile-020-trace-")); + const stub = makeStub( + [ + observation("bad2", { attrs: { kind: "auth_failure", secret: SECRET_MARKER }, fact: "provider_message" }), + EXPECTATION("ok3"), + { records_emitted: 0, status: "succeeded", type: "DONE" }, + ], + 0 + ); + try { + const outcome = await runStub(stub, "run_c020_trace", manifest(), { PDPP_TRACE_DIR: traceDir }); + assert.equal(outcome.ok, true, `run must succeed: ${outcome.ok ? "" : outcome.message}`); + const traceText = readdirSync(traceDir) + .map((name) => readFileSync(join(traceDir, name), "utf8")) + .join("\n"); + assert.ok( + traceText.includes("pdpp-runtime-trace"), + "the trace file must exist, so the absence below is meaningful" + ); + assert.equal(traceText.includes(SECRET_MARKER), false, "a rejected OBSERVATION must not be traced verbatim"); + assert.equal(traceText.includes('"ok3"'), false, "an accepted OBSERVATION must not be traced verbatim either"); + assert.ok(traceText.includes('"DONE"'), "non-OBSERVATION lines stay in the trace"); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + rmSync(traceDir, { force: true, recursive: true }); + } +}); + +test("a declared-profile run whose manifest names an unknown profile version never spawns the child", async () => { + const stub = makeStub([{ records_emitted: 0, status: "succeeded", type: "DONE" }], 0); + try { + const outcome = await runStub(stub, "run_c020_version", manifest({ protocol_version: "0.3.0" })); + assert.equal(outcome.ok, false, "an unknown profile version must be rejected before spawn"); + if (!outcome.ok) { + assert.match(outcome.message, PROTOCOL_VERSION_NAMED); + } + assert.equal(existsMarker(stub.markerPath), false, "the child must not execute"); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } +}); + +test("OBSERVATION or diagnostic_descriptors without protocol_version 0.2.0 are rejected before spawn", async (t) => { + await t.test("OBSERVATION capability on a 0.1.0 manifest", async () => { + const stub = makeStub([{ records_emitted: 0, status: "succeeded", type: "DONE" }], 0); + try { + const outcome = await runStub(stub, "run_c020_cap_010", manifest({ protocol_version: "0.1.0" })); + assert.equal(outcome.ok, false, "a 0.1.0 manifest may not declare OBSERVATION"); + if (!outcome.ok) { + assert.match(outcome.message, OBSERVATION_NEEDS_020); + } + assert.equal(existsMarker(stub.markerPath), false, "the child must not execute"); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } + }); + await t.test("diagnostic_descriptors on a 0.1.0 manifest", async () => { + const stub = makeStub([{ records_emitted: 0, status: "succeeded", type: "DONE" }], 0); + try { + const outcome = await runStub( + stub, + "run_c020_desc_010", + manifest({ protocol_capabilities: [], protocol_version: "0.1.0" }) + ); + assert.equal(outcome.ok, false, "diagnostic_descriptors require 0.2.0"); + if (!outcome.ok) { + assert.match(outcome.message, DIAGNOSTIC_DESCRIPTORS_NEEDS_020); + } + assert.equal(existsMarker(stub.markerPath), false, "the child must not execute"); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } + }); +}); + +test("an expectation naming an undeclared step is rejected before spawn", async () => { + const stub = makeStub([{ records_emitted: 0, status: "succeeded", type: "DONE" }], 0); + try { + const outcome = await runStub( + stub, + "run_c020_desc_step", + manifest({ diagnostic_descriptors: { expectations: [{ id: "email_input", step: "sign_in" }], steps: [] } }) + ); + assert.equal(outcome.ok, false, "an expectation on an undeclared step must be rejected"); + if (!outcome.ok) { + assert.match(outcome.message, UNDECLARED_STEP); + } + assert.equal(existsMarker(stub.markerPath), false, "the child must not execute"); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } +}); + +test("a failed DONE that cites an accepted fact yields a connector-attributed diagnosis and keeps the original text", async () => { + const original = "Sign-in was refused by the provider"; + const stub = makeStub( + [ + PROVIDER_REJECTED("p1"), + { + error: { basis: ["p1"], code: "credential_rejected", message: original, retryable: false }, + records_emitted: 0, + status: "failed", + type: "DONE", + }, + ], + 1 + ); + try { + const outcome = await runStub(stub, "run_c020_basis", manifest()); + assert.equal( + outcome.ok, + true, + `a failed DONE is a connector-reported result, not a runtime error: ${outcome.ok ? "" : outcome.message}` + ); + if (!outcome.ok) { + return; + } + assert.equal(outcome.result.status, "failed"); + assert.equal((outcome.result.connector_error as { message?: string } | null)?.message, original); + assert.equal((outcome.result.connector_error as { code?: string } | null)?.code, "credential_rejected"); + const record = await persistedRecord("run_c020_basis"); + assert.ok(record, "the diagnostic record must be persisted"); + assert.equal(record.diagnosis.category, "auth_rejected"); + assert.equal(record.diagnosis.attribution, "connector"); + assert.deepEqual(record.diagnosis.basis, ["p1"]); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } +}); + +test("uncited, retired and undeclared-rule facts cannot support a cause", async () => { + const stub = makeStub( + [ + PROVIDER_REJECTED("p2", "not_declared_rule"), + PROVIDER_REJECTED("p3"), + observation("p4", { attrs: { kind: "other" }, fact: "provider_message", resolves: ["p3"] }), + { + error: { basis: ["p2", "p3"], message: "Sign-in failed", retryable: false }, + records_emitted: 0, + status: "failed", + type: "DONE", + }, + ], + 1 + ); + try { + const outcome = await runStub(stub, "run_c020_nocause", manifest()); + assert.equal(outcome.ok, true, `run must resolve: ${outcome.ok ? "" : outcome.message}`); + const record = await persistedRecord("run_c020_nocause"); + assert.ok(record, "the diagnostic record must be persisted"); + assert.equal(record.diagnosis.category, "unknown", "an undeclared rule and a retired fact support nothing"); + assert.deepEqual(record.diagnosis.basis, []); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } +}); + +test("a DONE that cites an id the connector never emitted is rejected with no checkpoint commit", async () => { + const stub = makeStub( + [ + PROVIDER_REJECTED("p5"), + { + error: { basis: ["never_emitted"], message: "Sign-in failed", retryable: false }, + records_emitted: 0, + status: "failed", + type: "DONE", + }, + ], + 1 + ); + try { + const outcome = await runStub(stub, "run_c020_bad_basis", manifest()); + assert.equal(outcome.ok, false, "a DONE citing an id that was never emitted must fail the run"); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } +}); + +test("a malformed DONE recovery hint fails the run and keeps the connector message", async () => { + const original = "Provider unavailable"; + const stub = makeStub( + [ + { + error: { message: original, recovery_hint: { action: 42 }, retryable: true }, + records_emitted: 0, + status: "failed", + type: "DONE", + }, + ], + 1 + ); + try { + const outcome = await runStub(stub, "run_c020_bad_hint", manifest()); + assert.equal(outcome.ok, false, "a malformed hint is an invalid DONE"); + if (outcome.ok) { + return; + } + assert.ok(outcome.message.includes("DONE.error.recovery_hint"), `named rule: ${outcome.message}`); + assert.equal(outcome.connectorError?.message, original, "the bounded connector message stays visible"); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } +}); + +test("a failed 0.2.0 DONE without boolean retryable is rejected", async () => { + const stub = makeStub([{ error: { message: "boom" }, records_emitted: 0, status: "failed", type: "DONE" }], 1); + try { + const outcome = await runStub(stub, "run_c020_no_retryable", manifest()); + assert.equal(outcome.ok, false, "a 0.2.0 failed DONE needs a boolean retryable"); + if (!outcome.ok) { + assert.match(outcome.message, RETRYABLE_FIELD); + } + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } +}); + +test("a valid 0.1.0 run without OBSERVATION still succeeds", async () => { + const stub = makeStub( + [ + { message: "collecting", type: "PROGRESS" }, + { records_emitted: 0, status: "succeeded", type: "DONE" }, + ], + 0 + ); + try { + const outcome = await runStub( + stub, + "run_c020_legacy", + manifest({ diagnostic_descriptors: undefined, protocol_capabilities: undefined, protocol_version: "0.1.0" }) + ); + assert.equal(outcome.ok, true, `legacy run must succeed: ${outcome.ok ? "" : outcome.message}`); + if (!outcome.ok) { + return; + } + assert.equal(outcome.result.status, "succeeded"); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } +}); + +function existsMarker(path: string): boolean { + try { + readFileSync(path); + return true; + } catch { + return false; + } +} diff --git a/reference-implementation/test/collection-profile-owner-journey.test.ts b/reference-implementation/test/collection-profile-owner-journey.test.ts new file mode 100644 index 000000000..cfc215e6f --- /dev/null +++ b/reference-implementation/test/collection-profile-owner-journey.test.ts @@ -0,0 +1,108 @@ +// Copyright The PDP-Connect Contributors +// SPDX-License-Identifier: Apache-2.0 + +// Real HTTP/session + durable store boundary, beyond the route's fake adapter. +// Collection Profile 5.12: keep diagnostic records on the owner surface, +// uncached and namespace-scoped. +import assert from "node:assert/strict"; +import test from "node:test"; +import { closeDb, getDb } from "../server/db.ts"; +import { startServer } from "../server/index.ts"; +import { OWNER_SESSION_COOKIE_NAME } from "../server/owner-session.ts"; +import { + closePostgresStorage, + initPostgresStorage, + isPostgresStorageBackend, + postgresQuery, +} from "../server/postgres-storage.ts"; +import { createRequestConnectorInstanceStore } from "../server/request-store-factories.ts"; +import { admitOwnerRunConnection } from "../server/stores/connector-instance-store.ts"; +import { getDefaultConnectorObservationStore } from "../server/stores/connector-observation-store.ts"; +import { closeTestServer } from "./helpers/close-test-server.ts"; +import { dedicatedPostgresTestUrl } from "./helpers/dedicated-postgres-test-url.ts"; + +const POSTGRES_URL = dedicatedPostgresTestUrl(process.env.PDPP_TEST_POSTGRES_URL); +const OWNER_PASSWORD = "collection-profile-owner-fixture-password"; + +test("owner can read diagnostics through the real session gate", async () => { + if (POSTGRES_URL) { + await initPostgresStorage({ backend: "postgres", databaseUrl: POSTGRES_URL }); + } + const server = await startServer({ + asPort: 0, + autoEnrollEligibleSchedules: false, + ...(POSTGRES_URL ? { databaseUrl: POSTGRES_URL } : {}), + dbPath: ":memory:", + ownerAuthPassword: OWNER_PASSWORD, + quiet: true, + rsPort: 0, + storageBackend: POSTGRES_URL ? "postgres" : "sqlite", + }); + try { + assert.equal(isPostgresStorageBackend(), Boolean(POSTGRES_URL), "the HTTP fixture must use the selected backend"); + const origin = `http://localhost:${server.asPort}`; + const login = await fetch(`${origin}/owner/login`, { + body: JSON.stringify({ password: OWNER_PASSWORD }), + headers: { "Content-Type": "application/json" }, + method: "POST", + redirect: "manual", + }); + const cookie = login.headers + .getSetCookie() + .find((entry) => entry.startsWith(`${OWNER_SESSION_COOKIE_NAME}=`)) + ?.split(";")[0]; + assert.ok(cookie, "runtime-owned sign-in must issue a real owner session"); + const headers = { Accept: "application/json", Cookie: cookie }; + + const connectorId = "owner-observations-fixture"; + if (POSTGRES_URL) { + await postgresQuery("INSERT INTO connectors(connector_id, manifest) VALUES ($1, '{}'::jsonb)", [connectorId]); + } else { + getDb() + .prepare("INSERT INTO connectors(connector_id, manifest, created_at) VALUES (?, '{}', ?)") + .run(connectorId, "2026-11-02T00:00:00.000Z"); + } + const namespace = await admitOwnerRunConnection({ + connectorId, + connectorInstanceStore: createRequestConnectorInstanceStore(), + ownerSubjectId: "owner_local", + }); + const runId = "owner_diagnostic_run"; + await getDefaultConnectorObservationStore().put({ + connectorId, + connectorInstanceId: namespace.connectorInstanceId, + diagnosis: { attribution: "none", basis: [], category: "unknown", contributing: [] }, + runId, + snapshot: { defects: [], facts: [], truncated: false }, + }); + const query = new URLSearchParams({ + connector_id: connectorId, + connector_instance_id: namespace.connectorInstanceId, + }); + const url = `${origin}/_ref/runs/${runId}/observations?${query}`; + const denied = await fetch(url, { headers: { Accept: "application/json" }, redirect: "manual" }); + assert.equal(denied.status, 401, "anonymous callers cannot read diagnostics"); + const bearerDenied = await fetch(url, { + headers: { Accept: "application/json", Authorization: "Bearer non-owner-test-token" }, + redirect: "manual", + }); + assert.equal(bearerDenied.status, 401, "a bearer header does not replace the owner session"); + const read = await fetch(url, { headers }); + assert.equal(read.status, 200); + assert.equal(read.headers.get("cache-control"), "no-store"); + const body = (await read.json()) as { run_id: string; object: string; snapshot: unknown }; + assert.equal(body.object, "run_observations"); + assert.equal(body.run_id, runId); + assert.deepEqual(body.snapshot, { defects: [], facts: [], truncated: false }); + query.set("connector_instance_id", "other-connection"); + const wrongInstance = await fetch(`${origin}/_ref/runs/${runId}/observations?${query}`, { headers }); + assert.equal(wrongInstance.status, 404, "a run id cannot cross the connection namespace"); + } finally { + await closeTestServer(server); + if (POSTGRES_URL) { + await closePostgresStorage(); + } else { + closeDb(); + } + } +}); diff --git a/reference-implementation/test/collection-profile.test.ts b/reference-implementation/test/collection-profile.test.ts index eab00df4d..7437b7f49 100644 --- a/reference-implementation/test/collection-profile.test.ts +++ b/reference-implementation/test/collection-profile.test.ts @@ -8993,8 +8993,15 @@ rl.on('line', (line) => { } ); + // Oracle: Section 5.8 (DONE.error shape) and the owner-visibility rule in Section + // 5.11 (the connector's own DONE.error.message stays visible with the diagnosis). + // The run fails as connector_protocol_violation. The bounded message is kept, the + // unsupported field's VALUE never reaches durable artifacts, and the connector's + // retryable claim gives no retry authority. Plausible defect caught: an invalid + // envelope's unsupported value (or its retryable:true) leaking into the terminal + // record or restarting the run. Backend: memory profile, child process. await t.test( - "DONE.error with unsupported fields is rejected as a protocol violation before terminal artifacts are recorded as connector failures", + "DONE.error with unsupported fields is a protocol violation: the bounded connector message stays visible, the unsupported value and retryable claim do not", async () => { const server = await startTestServer({ asPort: 0, dbPath: ":memory:", quiet: true, rsPort: 0 }); const { asPort, rsPort } = server; @@ -9033,7 +9040,10 @@ rl.on('line', (line) => { assert.equal(checkpointSummaryOf(capturedError).buffered_records_dropped, 0); assert.equal(checkpointSummaryOf(capturedError).commit_status, "not_committed"); assert.equal(capturedError.terminal_reason, "connector_protocol_violation"); - assert.equal(capturedError.connector_error, null); + // The bounded connector message stays visible with the diagnosis (Section 5.11). + assert.equal(capturedError.connector_error?.message, "rate limited"); + // The connector's retryable:true is a claim with no retry authority. + assert.notEqual(capturedError.connector_error?.retryable, true); return true; } ); @@ -9047,13 +9057,13 @@ rl.on('line', (line) => { assert.equal(failedEvent.data.records_flushed, 0); assert.equal(failedEvent.data.buffered_records_dropped, 0); assert.equal(failedEvent.data.checkpoint_commit_status, "not_committed"); + // The connector's bounded message is durable diagnosis (Section 5.11). + assert.equal(failedEvent.data.connector_error_message, "rate limited"); + assert.notEqual(failedEvent.data.connector_error_retryable, true); + // The unsupported field's value must never reach a durable artifact. assert.ok( - !("connector_error_message" in failedEvent.data), - "unsupported DONE.error fields must not enter durable artifacts" - ); - assert.ok( - !("connector_error_retryable" in failedEvent.data), - "unsupported DONE.error fields must not enter durable artifacts" + !JSON.stringify(runTimeline).includes("upstream_rate_limit"), + "an unsupported DONE.error field's value must not enter durable artifacts" ); } finally { cleanup(); @@ -9062,8 +9072,11 @@ rl.on('line', (line) => { } ); + // Section 5.8: successful DONE cannot carry a terminal connector error. This + // contradiction is a protocol failure and must not be published as a valid + // connector error. Failed-DONE message preservation is tested separately. await t.test( - "DONE(succeeded) with terminal error details is rejected as a protocol violation before success artifacts are recorded", + "DONE(succeeded) with terminal error details is a protocol violation before success artifacts are recorded", async () => { const server = await startTestServer({ asPort: 0, dbPath: ":memory:", quiet: true, rsPort: 0 }); const { asPort, rsPort } = server; @@ -9120,14 +9133,8 @@ rl.on('line', (line) => { assert.equal(failedEvent.data.records_flushed, 0); assert.equal(failedEvent.data.buffered_records_dropped, 0); assert.equal(failedEvent.data.checkpoint_commit_status, "not_committed"); - assert.ok( - !("connector_error_message" in failedEvent.data), - "contradictory DONE(succeeded)+error details must not enter durable artifacts" - ); - assert.ok( - !("connector_error_retryable" in failedEvent.data), - "contradictory DONE(succeeded)+error details must not enter durable artifacts" - ); + assert.ok(!("connector_error_message" in failedEvent.data)); + assert.ok(!("connector_error_retryable" in failedEvent.data)); } finally { cleanup(); await closeServer(server); diff --git a/reference-implementation/test/connector-observation-store.test.ts b/reference-implementation/test/connector-observation-store.test.ts new file mode 100644 index 000000000..59b9e0f4d --- /dev/null +++ b/reference-implementation/test/connector-observation-store.test.ts @@ -0,0 +1,843 @@ +// Copyright The PDP-Connect Contributors +// SPDX-License-Identifier: Apache-2.0 + +/** + * Connector observation run store (Collection Profile v0.2.0, data-connectors + * spec sections 5.10.3 and 5.10.4) on both backends. + * + * Fixtures come from the typed runtime (`createObservationRun`), so retired + * facts and runtime defects are produced by the same code that the runtime + * uses, not hand-built. The Postgres cases skip unless PDPP_TEST_POSTGRES_URL + * is set, as in checkpoint-withhold-store.test.ts. + */ + +import assert from "node:assert/strict"; +import { randomUUID } from "node:crypto"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import test from "node:test"; +import { + createObservationRun, + type Diagnosis, + type ObservationSnapshot, + validateDiagnosticDescriptors, +} from "../runtime/observations.ts"; +import { closeDb, getDb, initDb } from "../server/db.ts"; +import { closePostgresStorage, initPostgresStorage, postgresQuery } from "../server/postgres-storage.ts"; +import { + CONNECTOR_OBSERVATION_RECORD_MAX_BYTES, + CONNECTOR_OBSERVATION_RUNS_PER_CONNECTOR, + type ConnectorObservationRunInput, + type ConnectorObservationRunStore, + ConnectorObservationStoreError, + getDefaultConnectorObservationStore, + resetDefaultConnectorObservationStoreCache, +} from "../server/stores/connector-observation-store.ts"; +import { dedicatedPostgresTestUrl } from "./helpers/dedicated-postgres-test-url.ts"; + +const POSTGRES_URL = dedicatedPostgresTestUrl(process.env.PDPP_TEST_POSTGRES_URL); +const NOW = "2026-11-02T00:00:00.000Z"; +const CAP = CONNECTOR_OBSERVATION_RUNS_PER_CONNECTOR; +const PRUNE_FAULT = /seeded prune fault/; +const INSERT_FAULT = /seeded insert fault/; + +type FaultKind = "prune" | "insert"; + +/** One storage engine under test. Both implementations expose the same seams. */ +interface Harness { + deleteConnection: (connectorInstanceId: string) => Promise; + installFault: (kind: FaultKind) => Promise; + removeFault: (kind: FaultKind) => Promise; + reopen: () => Promise; + retainedSequences: (connectorId: string) => Promise; + rowCount: (connectorId: string) => Promise; + seedConnection: (connectorId: string, connectorInstanceId: string) => Promise; +} + +function uniq(tag: string): string { + return `${tag}-${randomUUID()}`; +} + +function pad(n: number): string { + return String(n).padStart(2, "0"); +} + +/** Runs a synchronous SQLite step; a throw becomes a rejection, as the async Postgres seam does. */ +function settle(step: () => T): Promise { + try { + return Promise.resolve(step()); + } catch (error) { + return Promise.reject(error); + } +} + +/** + * Runs `step` for each item one at a time, in list order. Store sequence numbers + * follow call order, so the writes must not interleave. A plain `await` loop is + * flagged by noAwaitInLoops, and Promise.all would interleave them. + */ +function inOrder(items: readonly T[], step: (item: T) => Promise): Promise { + return items + .reduce>((done, item) => done.then(() => step(item)), Promise.resolve()) + .then(() => undefined); +} + +/** Runtime-built snapshot: one retired fact, one auth failure, one runtime defect. */ +function observedRun(tag: string): { + snapshot: ObservationSnapshot; + diagnosis: Diagnosis; +} { + const run = createObservationRun({ + declared: true, + descriptors: validateDiagnosticDescriptors({ + expectations: [{ id: "login_button", step: "login" }], + rules: [{ id: "bad_password", kind: "auth", step: "login" }], + steps: [{ id: "login" }], + }), + }); + const expect = run.accept({ + attrs: { + expectation: "login_button", + final: "absent", + states_seen: ["absent"], + }, + fact: "element_expectation", + id: `${tag}-expect`, + step: "login", + type: "OBSERVATION", + }); + const auth = run.accept({ + attrs: { kind: "auth_failure", rule: "bad_password" }, + fact: "provider_message", + id: `${tag}-auth`, + step: "login", + type: "OBSERVATION", + }); + const retry = run.accept({ + attrs: { + expectation: "login_button", + final: "matched", + states_seen: ["absent", "matched"], + }, + fact: "element_expectation", + id: `${tag}-retry`, + resolves: [`${tag}-expect`], + step: "login", + type: "OBSERVATION", + }); + // A runtime fact type from a connector is discarded and recorded as a defect. + run.accept({ fact: "rule_match", id: `${tag}-forged`, type: "OBSERVATION" }); + for (const outcome of [expect, auth, retry]) { + assert.equal(outcome.status, "accepted", "runtime fixture fact must be accepted"); + } + return { + diagnosis: run.diagnose([`${tag}-auth`, `${tag}-expect`]), + snapshot: { + ...run.snapshot(), + connector_suggestions: [{ action: "refresh_credentials", attribution: "connector", origin: "done_error" }], + }, + }; +} + +function runInput(connectorId: string, connectorInstanceId: string, runId: string): ConnectorObservationRunInput { + return { connectorId, connectorInstanceId, runId, ...observedRun(runId) }; +} + +function storeOf(): ConnectorObservationRunStore { + return getDefaultConnectorObservationStore(); +} + +async function expectStoreError(promise: Promise, code: string, message: string): Promise { + await assert.rejects(promise, (error: unknown) => { + assert.ok(error instanceof ConnectorObservationStoreError, `${message}: expected ConnectorObservationStoreError`); + assert.equal(error.code, code, message); + return true; + }); +} + +function faultSql(kind: FaultKind, action: "install" | "remove"): string[] { + const label = kind === "prune" ? "seeded prune fault" : "seeded insert fault"; + const event = kind === "prune" ? "DELETE" : "INSERT"; + const name = `observation_${kind}_fault`; + if (action === "remove") { + return [`DROP TRIGGER IF EXISTS ${name}`]; + } + return [ + `CREATE TRIGGER ${name} BEFORE ${event} ON connector_observation_runs BEGIN SELECT RAISE(ABORT, '${label}'); END`, + ]; +} + +function sqliteHarness(path: string): Harness { + return { + deleteConnection: (connectorInstanceId) => + settle(() => { + getDb().prepare("DELETE FROM connector_instances WHERE connector_instance_id = ?").run(connectorInstanceId); + }), + installFault: (kind) => + settle(() => { + for (const sql of faultSql(kind, "install")) { + getDb().exec(sql); + } + }), + removeFault: (kind) => + settle(() => { + for (const sql of faultSql(kind, "remove")) { + getDb().exec(sql); + } + }), + reopen: () => + settle(() => { + closeDb(); + initDb(path); + resetDefaultConnectorObservationStoreCache(); + }), + retainedSequences: (connectorId) => + settle(() => { + const rows = getDb() + .prepare( + "SELECT recorded_seq AS seq FROM connector_observation_runs WHERE connector_id = ? ORDER BY recorded_seq" + ) + .all(connectorId) as { seq: number }[]; + return rows.map((row) => Number(row.seq)); + }), + rowCount: (connectorId) => + settle(() => { + const row = getDb() + .prepare("SELECT COUNT(*) AS n FROM connector_observation_runs WHERE connector_id = ?") + .get(connectorId) as { n: number }; + return row.n; + }), + seedConnection: (connectorId, connectorInstanceId) => + settle(() => { + getDb() + .prepare("INSERT OR IGNORE INTO connectors(connector_id, manifest, created_at) VALUES (?, '{}', ?)") + .run(connectorId, NOW); + getDb() + .prepare( + `INSERT OR IGNORE INTO connector_instances( + connector_instance_id, owner_subject_id, connector_id, display_name, status, + source_kind, source_binding_key, source_binding_json, created_at, updated_at, revoked_at + ) VALUES (?, 'owner_local', ?, 'x', 'active', 'account', ?, '{}', ?, ?, NULL)` + ) + .run(connectorInstanceId, connectorId, connectorInstanceId, NOW, NOW); + }), + }; +} + +function postgresHarness(databaseUrl: string): Harness { + return { + async deleteConnection(connectorInstanceId) { + await postgresQuery("DELETE FROM connector_instances WHERE connector_instance_id = $1", [connectorInstanceId]); + }, + async installFault(kind) { + const label = kind === "prune" ? "seeded prune fault" : "seeded insert fault"; + const event = kind === "prune" ? "DELETE" : "INSERT"; + const fn = `observation_${kind}_fault_fn`; + await postgresQuery( + `CREATE OR REPLACE FUNCTION ${fn}() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN RAISE EXCEPTION '${label}'; END $$` + ); + await postgresQuery( + `CREATE TRIGGER observation_${kind}_fault BEFORE ${event} ON connector_observation_runs FOR EACH STATEMENT EXECUTE FUNCTION ${fn}()` + ); + }, + async removeFault(kind) { + await postgresQuery(`DROP TRIGGER IF EXISTS observation_${kind}_fault ON connector_observation_runs`); + await postgresQuery(`DROP FUNCTION IF EXISTS observation_${kind}_fault_fn()`); + }, + async reopen() { + await closePostgresStorage(); + await initPostgresStorage({ backend: "postgres", databaseUrl }); + resetDefaultConnectorObservationStoreCache(); + }, + async retainedSequences(connectorId) { + const result = await postgresQuery<{ seq: string | number }>( + "SELECT recorded_seq AS seq FROM connector_observation_runs WHERE connector_id = $1 ORDER BY recorded_seq", + [connectorId] + ); + return result.rows.map((row) => Number(row.seq)); + }, + async rowCount(connectorId) { + const result = await postgresQuery<{ n: number }>( + "SELECT COUNT(*)::int AS n FROM connector_observation_runs WHERE connector_id = $1", + [connectorId] + ); + return result.rows[0]?.n ?? 0; + }, + async seedConnection(connectorId, connectorInstanceId) { + await postgresQuery( + "INSERT INTO connectors(connector_id, manifest) VALUES ($1, $2::jsonb) ON CONFLICT DO NOTHING", + [connectorId, "{}"] + ); + await postgresQuery( + `INSERT INTO connector_instances( + connector_instance_id, owner_subject_id, connector_id, display_name, status, + source_kind, source_binding_key, source_binding_json, created_at, updated_at, revoked_at + ) VALUES ($1, 'owner_local', $2, 'x', 'active', 'account', $1, '{}'::jsonb, $3, $3, NULL) + ON CONFLICT DO NOTHING`, + [connectorInstanceId, connectorId, NOW] + ); + }, + }; +} + +async function withSqlite(fn: (harness: Harness) => Promise): Promise { + const dir = mkdtempSync(join(tmpdir(), "pdpp-observation-store-")); + const path = join(dir, "pdpp.sqlite"); + resetDefaultConnectorObservationStoreCache(); + initDb(path); + try { + await fn(sqliteHarness(path)); + } finally { + closeDb(); + resetDefaultConnectorObservationStoreCache(); + rmSync(dir, { force: true, recursive: true }); + } +} + +async function withPostgres(fn: (harness: Harness) => Promise): Promise { + const databaseUrl = POSTGRES_URL ?? ""; + await initPostgresStorage({ backend: "postgres", databaseUrl }); + resetDefaultConnectorObservationStoreCache(); + try { + await fn(postgresHarness(databaseUrl)); + } finally { + await closePostgresStorage(); + resetDefaultConnectorObservationStoreCache(); + } +} + +/** Seeds one connection per (connector, instance) pair, then writes runs for it. */ +async function seedAndPut( + harness: Harness, + connectorId: string, + connectorInstanceId: string, + runIds: readonly string[] +): Promise { + await harness.seedConnection(connectorId, connectorInstanceId); + await inOrder(runIds, (runId) => storeOf().put(runInput(connectorId, connectorInstanceId, runId))); +} + +async function contractPersistsExactSnapshot(harness: Harness): Promise { + const c = uniq("c-persist"); + const i = uniq("i-persist"); + await harness.seedConnection(c, i); + const input = runInput(c, i, "run-persist"); + await storeOf().put(input); + + const expected = JSON.parse(JSON.stringify(input)) as ConnectorObservationRunInput; + const readBack = await storeOf().get({ + connectorId: c, + connectorInstanceId: i, + runId: "run-persist", + }); + assert.ok(readBack, "the record persists in the same process"); + assert.deepEqual(readBack.snapshot, expected.snapshot, "snapshot reads back exactly"); + assert.deepEqual(readBack.diagnosis, expected.diagnosis, "diagnosis reads back exactly"); + assert.equal(readBack.runId, "run-persist"); + assert.equal(readBack.connectorId, c); + assert.equal(readBack.connectorInstanceId, i); + + await harness.reopen(); + const afterReopen = await storeOf().get({ + connectorId: c, + connectorInstanceId: i, + runId: "run-persist", + }); + assert.deepEqual(afterReopen?.snapshot, expected.snapshot, "snapshot survives a reopen unchanged"); + assert.deepEqual(afterReopen?.diagnosis, expected.diagnosis, "diagnosis survives a reopen unchanged"); + + const retired = readBack.snapshot.facts.filter((fact) => fact.retired); + assert.deepEqual( + retired.map((fact) => fact.id), + ["run-persist-expect"], + "the retirement flag set by the runtime is read back as supplied" + ); + assert.deepEqual( + readBack.snapshot.defects.map((defect) => defect.class), + ["protocol_violation"], + "the runtime defect is read back as supplied" + ); +} + +async function contractNamespaceAndInstanceSeparation(harness: Harness): Promise { + const c1 = uniq("c1"); + const c2 = uniq("c2"); + const i1 = uniq("i1"); + const i2 = uniq("i2"); + const i3 = uniq("i3"); + await harness.seedConnection(c1, i1); + await harness.seedConnection(c1, i2); + await harness.seedConnection(c2, i3); + await storeOf().put(runInput(c1, i1, "shared")); + await storeOf().put(runInput(c1, i2, "shared")); + await storeOf().put(runInput(c2, i3, "shared")); + + const fromI1 = await storeOf().get({ + connectorId: c1, + connectorInstanceId: i1, + runId: "shared", + }); + const fromI2 = await storeOf().get({ + connectorId: c1, + connectorInstanceId: i2, + runId: "shared", + }); + assert.equal(fromI1?.connectorInstanceId, i1, "exact instance lookup returns its own record"); + assert.equal(fromI2?.connectorInstanceId, i2, "a sibling instance with the same run id is a separate record"); + assert.equal( + await storeOf().get({ + connectorId: c1, + connectorInstanceId: uniq("i-missing"), + runId: "shared", + }), + null, + "an instance that did not write the run is not returned" + ); + assert.equal( + await storeOf().get({ + connectorId: c2, + connectorInstanceId: i1, + runId: "shared", + }), + null, + "a different connector cannot read another connector's record by instance" + ); + + const c1All = await storeOf().list({ connectorId: c1 }); + assert.equal(c1All.length, 2, "list without an instance covers every instance of the connector"); + const c1OnlyI1 = await storeOf().list({ + connectorId: c1, + connectorInstanceId: i1, + }); + assert.deepEqual( + c1OnlyI1.map((record) => record.connectorInstanceId), + [i1], + "list with an instance returns only that instance" + ); + const c2Only = await storeOf().list({ connectorId: c2 }); + assert.deepEqual( + c2Only.map((record) => record.connectorId), + [c2], + "list never returns another connector's records" + ); +} + +async function contractRetentionIsPerConnector(harness: Harness): Promise { + const c1 = uniq("c1-retain"); + const c2 = uniq("c2-retain"); + const i1 = uniq("i1-retain"); + const i2 = uniq("i2-retain"); + const i3 = uniq("i3-retain"); + await harness.seedConnection(c1, i1); + await harness.seedConnection(c1, i2); + await harness.seedConnection(c2, i3); + await inOrder( + Array.from({ length: CAP + 2 }, (_, index) => index + 1), + (n) => { + const instance = n % 2 === 1 ? i1 : i2; + return storeOf().put(runInput(c1, instance, `run-${pad(n)}`)); + } + ); + await inOrder( + Array.from({ length: 3 }, (_, index) => index + 1), + (n) => storeOf().put(runInput(c2, i3, `run-${pad(n)}`)) + ); + + const expectedRuns = Array.from({ length: CAP }, (_, index) => `run-${pad(CAP + 2 - index)}`); + assert.deepEqual( + (await storeOf().list({ connectorId: c1 })).map((record) => record.runId), + expectedRuns, + "connector c1 retains exactly its 8 newest runs, newest first" + ); + assert.equal(await harness.rowCount(c1), CAP, "the 8-record cap counts runs across both instances"); + assert.deepEqual( + (await storeOf().list({ connectorId: c1, connectorInstanceId: i1 })).map((record) => record.runId), + ["run-09", "run-07", "run-05", "run-03"], + "instance i1 holds only its own retained runs" + ); + assert.equal( + await storeOf().get({ + connectorId: c1, + connectorInstanceId: i1, + runId: "run-01", + }), + null, + "the oldest run of connector c1 is pruned" + ); + assert.equal( + await storeOf().get({ + connectorId: c1, + connectorInstanceId: i2, + runId: "run-02", + }), + null, + "a pruned run of the sibling instance is pruned too" + ); + assert.equal(await harness.rowCount(c2), 3, "pruning connector c1 leaves connector c2 untouched"); + assert.ok( + await storeOf().get({ + connectorId: c2, + connectorInstanceId: i3, + runId: "run-01", + }), + "connector c2 keeps its oldest record" + ); +} + +async function contractOversizeRejectedBeforeMutation(harness: Harness): Promise { + const c = uniq("c-oversize"); + const i = uniq("i-oversize"); + await seedAndPut( + harness, + c, + i, + Array.from({ length: CAP }, (_, index) => `run-${pad(index + 1)}`) + ); + const before = await harness.retainedSequences(c); + const base = observedRun("oversize"); + const oversize = { + connectorId: c, + connectorInstanceId: i, + diagnosis: base.diagnosis, + runId: "run-oversize", + snapshot: { + ...base.snapshot, + facts: [ + ...base.snapshot.facts, + { + attrs: { + kind: "other", + pad: "x".repeat(CONNECTOR_OBSERVATION_RECORD_MAX_BYTES), + }, + fact: "provider_message", + id: "pad", + type: "OBSERVATION", + }, + ], + }, + } as unknown as ConnectorObservationRunInput; + + await expectStoreError(storeOf().put(oversize), "record_oversize", "an oversize record is rejected"); + assert.equal(await harness.rowCount(c), CAP, "an oversize write does not change the row count"); + assert.deepEqual(await harness.retainedSequences(c), before, "an oversize write does not prune or renumber"); + assert.equal( + await storeOf().get({ + connectorId: c, + connectorInstanceId: i, + runId: "run-oversize", + }), + null, + "the rejected record is not stored" + ); + assert.ok( + await storeOf().get({ + connectorId: c, + connectorInstanceId: i, + runId: "run-01", + }), + "the oldest retained run survives a rejected write" + ); +} + +async function contractRepeatPutIsIdempotent(harness: Harness): Promise { + const c1 = uniq("c1-repeat"); + const c2 = uniq("c2-repeat"); + const i1 = uniq("i1-repeat"); + const i3 = uniq("i3-repeat"); + await harness.seedConnection(c1, i1); + await harness.seedConnection(c2, i3); + const original = runInput(c1, i1, "dup"); + await storeOf().put(original); + await storeOf().put(original); + assert.equal(await harness.rowCount(c1), 1, "a repeat put of the same run does not grow the row count"); + + const conflicting: ConnectorObservationRunInput = { + ...original, + diagnosis: { + attribution: "none", + basis: [], + category: "unknown", + contributing: [], + }, + }; + await expectStoreError( + storeOf().put(conflicting), + "record_conflict", + "different content for the same run is rejected" + ); + const stored = await storeOf().get({ + connectorId: c1, + connectorInstanceId: i1, + runId: "dup", + }); + assert.equal(stored?.diagnosis.category, original.diagnosis.category, "the stored record keeps its original content"); + + const other = runInput(c2, i3, "dup"); + await storeOf().put(other); + const c1After = await storeOf().get({ + connectorId: c1, + connectorInstanceId: i1, + runId: "dup", + }); + assert.deepEqual(c1After?.snapshot, original.snapshot, "a same-id run in another connector does not overwrite c1"); + assert.equal( + ( + await storeOf().get({ + connectorId: c2, + connectorInstanceId: i3, + runId: "dup", + }) + )?.connectorId, + c2, + "the other connector gets its own record" + ); +} + +async function contractValidationAndLimit(harness: Harness): Promise { + const c = uniq("c-valid"); + const i = uniq("i-valid"); + await seedAndPut( + harness, + c, + i, + Array.from({ length: CAP + 2 }, (_, index) => `run-${pad(index + 1)}`) + ); + + await expectStoreError( + storeOf().put({ ...runInput(c, i, "run-blank"), runId: " " }), + "record_invalid", + "a blank run id is rejected" + ); + const missingFacts = { + ...runInput(c, i, "run-bad"), + snapshot: { defects: [], truncated: false }, + } as unknown as ConnectorObservationRunInput; + await expectStoreError(storeOf().put(missingFacts), "record_invalid", "a snapshot without facts is rejected"); + await inOrder( + [ + { action: "refresh_credentials", attribution: "runtime", origin: "done_error" }, + { action: "arbitrary_action", attribution: "connector", origin: "done_error" }, + { action: "refresh_credentials", attribution: "connector", origin: "arbitrary_origin" }, + { action: "refresh_credentials", attribution: "connector", message: "private text", origin: "done_error" }, + ], + (suggestion) => { + const input = runInput(c, i, "invalid-suggestion"); + return expectStoreError( + storeOf().put({ + ...input, + snapshot: { ...input.snapshot, connector_suggestions: [suggestion] }, + } as unknown as ConnectorObservationRunInput), + "record_invalid", + "only closed connector-attributed suggestion fields may be persisted" + ); + } + ); + const excessiveSuggestions = runInput(c, i, "excessive-suggestions"); + await expectStoreError( + storeOf().put({ + ...excessiveSuggestions, + snapshot: { + ...excessiveSuggestions.snapshot, + connector_suggestions: Array.from({ length: 17 }, () => ({ + action: "refresh_credentials", + attribution: "connector", + origin: "done_error", + })), + }, + } as unknown as ConnectorObservationRunInput), + "record_invalid", + "suggestion storage is bounded by the sixteen portable action and origin pairs" + ); + assert.equal(await harness.rowCount(c), CAP, "rejected records do not change the stored rows"); + + assert.equal((await storeOf().list({ connectorId: c, limit: 100 })).length, CAP, "list limit clamps to 8"); + assert.equal((await storeOf().list({ connectorId: c, limit: 1 })).length, 1, "list honors a smaller limit"); + await assert.rejects( + storeOf().list({ connectorId: c, limit: 0 }), + ConnectorObservationStoreError, + "limit 0 is rejected" + ); +} + +async function contractDeletingConnectionCascades(harness: Harness): Promise { + const c = uniq("c-cascade"); + const deleted = uniq("i-cascade-deleted"); + const sibling = uniq("i-cascade-sibling"); + await seedAndPut(harness, c, deleted, ["run-01", "run-02"]); + await harness.seedConnection(c, sibling); + await storeOf().put(runInput(c, sibling, "run-03")); + assert.equal(await harness.rowCount(c), 3, "three runs are recorded before the deletion"); + + await harness.deleteConnection(deleted); + assert.equal( + await storeOf().get({ + connectorId: c, + connectorInstanceId: deleted, + runId: "run-01", + }), + null, + "deleting a connection removes its run records through the foreign key" + ); + assert.equal(await harness.rowCount(c), 1, "only the deleted connection's records are removed"); + assert.ok( + await storeOf().get({ + connectorId: c, + connectorInstanceId: sibling, + runId: "run-03", + }), + "a sibling instance of the same connector keeps its record" + ); +} + +async function contractRollsBackOnFault(harness: Harness): Promise { + const c = uniq("c-fault"); + const i = uniq("i-fault"); + await seedAndPut( + harness, + c, + i, + Array.from({ length: CAP }, (_, index) => `run-${pad(index + 1)}`) + ); + const before = await harness.retainedSequences(c); + + await harness.installFault("prune"); + try { + await assert.rejects(storeOf().put(runInput(c, i, "run-09")), PRUNE_FAULT, "a prune fault fails the write"); + } finally { + await harness.removeFault("prune"); + } + assert.equal( + await storeOf().get({ + connectorId: c, + connectorInstanceId: i, + runId: "run-09", + }), + null, + "the insert is rolled back when its prune fails" + ); + assert.deepEqual(await harness.retainedSequences(c), before, "a rolled-back prune leaves sequences unchanged"); + + await harness.installFault("insert"); + try { + await assert.rejects(storeOf().put(runInput(c, i, "run-09")), INSERT_FAULT, "an insert fault fails the write"); + } finally { + await harness.removeFault("insert"); + } + assert.equal(await harness.rowCount(c), CAP, "a failed insert leaves the retained rows as they were"); + assert.deepEqual(await harness.retainedSequences(c), before, "a failed insert leaves sequences unchanged"); + + await storeOf().put(runInput(c, i, "run-09")); + assert.equal(await harness.rowCount(c), CAP, "after the faults are removed the write succeeds and prunes once"); +} + +async function contractConcurrentPutsKeepRetention(harness: Harness): Promise { + const c = uniq("c-race"); + const other = uniq("c-race-other"); + const instances = [uniq("i-race-a"), uniq("i-race-b"), uniq("i-race-c")]; + await inOrder(instances, (instance) => harness.seedConnection(c, instance)); + const otherInstance = uniq("i-race-other"); + await harness.seedConnection(other, otherInstance); + const total = 3 * CAP; + const writes = Array.from({ length: total }, (_, index) => + storeOf().put(runInput(c, instances[index % instances.length] ?? "", `run-${pad(index + 1)}`)) + ); + await Promise.all(writes); + + assert.equal(await harness.rowCount(c), CAP, "concurrent writers leave exactly 8 records for the connector"); + assert.deepEqual( + await harness.retainedSequences(c), + Array.from({ length: CAP }, (_, index) => total - CAP + 1 + index), + "every write took a distinct sequence and the newest 8 survived" + ); + + const same = runInput(other, otherInstance, "same"); + await Promise.all([storeOf().put(same), storeOf().put(same)]); + assert.equal(await harness.rowCount(other), 1, "concurrent identical puts store one row"); +} + +test("SQLite connector observation store: persists, reopens and reads back the exact snapshot", async () => { + await withSqlite(contractPersistsExactSnapshot); +}); + +test("SQLite connector observation store: separates connector namespace and exact instance", async () => { + await withSqlite(contractNamespaceAndInstanceSeparation); +}); + +test("SQLite connector observation store: retains 8 runs per connector across instances", async () => { + await withSqlite(contractRetentionIsPerConnector); +}); + +test("SQLite connector observation store: rejects an oversize record before any mutation", async () => { + await withSqlite(contractOversizeRejectedBeforeMutation); +}); + +test("SQLite connector observation store: a repeat put is idempotent and never takes another connector's run", async () => { + await withSqlite(contractRepeatPutIsIdempotent); +}); + +test("SQLite connector observation store: validates identity and clamps list limit to 8", async () => { + await withSqlite(contractValidationAndLimit); +}); + +test("SQLite connector observation store: rolls back the write when the prune or insert faults", async () => { + await withSqlite(contractRollsBackOnFault); +}); + +test("SQLite connector observation store: deleting a connection cascades its run records", async () => { + await withSqlite(contractDeletingConnectionCascades); +}); + +test("Postgres connector observation store: persists, reopens and reads back the exact snapshot", { + skip: POSTGRES_URL ? false : "PDPP_TEST_POSTGRES_URL unset", +}, async () => { + await withPostgres(contractPersistsExactSnapshot); +}); + +test("Postgres connector observation store: separates connector namespace and exact instance", { + skip: POSTGRES_URL ? false : "PDPP_TEST_POSTGRES_URL unset", +}, async () => { + await withPostgres(contractNamespaceAndInstanceSeparation); +}); + +test("Postgres connector observation store: retains 8 runs per connector across instances", { + skip: POSTGRES_URL ? false : "PDPP_TEST_POSTGRES_URL unset", +}, async () => { + await withPostgres(contractRetentionIsPerConnector); +}); + +test("Postgres connector observation store: rejects an oversize record before any mutation", { + skip: POSTGRES_URL ? false : "PDPP_TEST_POSTGRES_URL unset", +}, async () => { + await withPostgres(contractOversizeRejectedBeforeMutation); +}); + +test("Postgres connector observation store: a repeat put is idempotent and never takes another connector's run", { + skip: POSTGRES_URL ? false : "PDPP_TEST_POSTGRES_URL unset", +}, async () => { + await withPostgres(contractRepeatPutIsIdempotent); +}); + +test("Postgres connector observation store: validates identity and clamps list limit to 8", { + skip: POSTGRES_URL ? false : "PDPP_TEST_POSTGRES_URL unset", +}, async () => { + await withPostgres(contractValidationAndLimit); +}); + +test("Postgres connector observation store: rolls back the write when the prune or insert faults", { + skip: POSTGRES_URL ? false : "PDPP_TEST_POSTGRES_URL unset", +}, async () => { + await withPostgres(contractRollsBackOnFault); +}); + +test("Postgres connector observation store: concurrent puts keep retention and unique sequences", { + skip: POSTGRES_URL ? false : "PDPP_TEST_POSTGRES_URL unset", +}, async () => { + await withPostgres(contractConcurrentPutsKeepRetention); +}); + +test("Postgres connector observation store: deleting a connection cascades its run records", { + skip: POSTGRES_URL ? false : "PDPP_TEST_POSTGRES_URL unset", +}, async () => { + await withPostgres(contractDeletingConnectionCascades); +}); diff --git a/reference-implementation/test/detail-coverage-shortfall-severity.test.ts b/reference-implementation/test/detail-coverage-shortfall-severity.test.ts index cf9721146..ea19e9967 100644 --- a/reference-implementation/test/detail-coverage-shortfall-severity.test.ts +++ b/reference-implementation/test/detail-coverage-shortfall-severity.test.ts @@ -1290,8 +1290,8 @@ test("the pre-spawn gate rejects unknown and unsupported required capabilities a [["DETAIL_COVERAGE_KEY_SETS", "FORGED"], UNKNOWN_CAPABILITY_PATTERN, null], ["DETAIL_COVERAGE_KEY_SETS", UNKNOWN_CAPABILITY_PATTERN, null], [null, UNKNOWN_CAPABILITY_PATTERN, null], - [["OBSERVATION"], UNSUPPORTED_CAPABILITY_PATTERN, "OBSERVATION"], - [["DETAIL_COVERAGE_KEY_SETS", "OBSERVATION", "BLOB"], UNSUPPORTED_CAPABILITY_PATTERN, "OBSERVATION"], + [["OBSERVATION"], null, null], + [["DETAIL_COVERAGE_KEY_SETS", "OBSERVATION", "BLOB"], null, null], [["DETAIL_COVERAGE_KEY_SETS", "STREAM_EVIDENCE"], null, null], ]; for (const [protocolCapabilities, rejection, unsupported] of cases) { @@ -1319,7 +1319,8 @@ test("the pre-spawn gate rejects unknown and unsupported required capabilities a // START.protocol_capabilities lists what this runtime implements (Section // 5.1): it accepts empty_keys/unchanged_keys and it validates STREAM_EVIDENCE. -// It does not implement BLOB or OBSERVATION, so it does not list them. +// It also accepts OBSERVATION (Section 5.10). It does not implement BLOB, so it +// does not list it. test("START advertises the protocol capabilities this runtime implements, to every connector", async (t) => { const server = await typedStartServer({ asPort: 0, dbPath: ":memory:", quiet: true, rsPort: 0 }); t.after(() => closeServer(server)); @@ -1333,7 +1334,7 @@ test("START advertises the protocol capabilities this runtime implements, to eve holdingsRun([holdingsCoverage({ empty_keys: ["K"], required_keys: ["K"] })]) ); assert.equal(thrown, null); - assert.deepEqual(start?.protocol_capabilities, ["DETAIL_COVERAGE_KEY_SETS", "STREAM_EVIDENCE"]); + assert.deepEqual(start?.protocol_capabilities, ["DETAIL_COVERAGE_KEY_SETS", "OBSERVATION", "STREAM_EVIDENCE"]); } }); diff --git a/reference-implementation/test/done-error-normalization-oracle.test.ts b/reference-implementation/test/done-error-normalization-oracle.test.ts index 4adea9335..7b7fd1d4e 100644 --- a/reference-implementation/test/done-error-normalization-oracle.test.ts +++ b/reference-implementation/test/done-error-normalization-oracle.test.ts @@ -20,6 +20,17 @@ test("validateDoneError returns null when DONE.error is absent", () => { assert.equal(validateDoneError("failed", null), null); }); +test("portable DONE basis can cite more than 64 earlier observations without inventing a schema limit", () => { + // Sections 5.8 and 5.10 allow a basis over emitted ids; 64 is the minimum + // supported observation count, not a maximum basis length. + const basis = Array.from({ length: 65 }, (_, index) => `observation_${index}`); + const result = validateDoneError("failed", { message: "failed", retryable: false, basis }, { + strict: true, observationDeclared: true, hasEmittedId: (id) => basis.includes(id), + }); + assert.ok(!(result instanceof Error), "valid earlier ids must not be rejected by an invented 64-id cap"); + assert.deepEqual(result?.basis, basis); +}); + test("validateDoneError rejects terminal error details on succeeded DONE envelopes", () => { const result = validateDoneError("succeeded", { message: "failed anyway" }); diff --git a/reference-implementation/test/helpers/ri-zero-connector-knowledge-data-load-scan.ts b/reference-implementation/test/helpers/ri-zero-connector-knowledge-data-load-scan.ts index 17e7a0ae2..17c2a2ea3 100644 --- a/reference-implementation/test/helpers/ri-zero-connector-knowledge-data-load-scan.ts +++ b/reference-implementation/test/helpers/ri-zero-connector-knowledge-data-load-scan.ts @@ -150,12 +150,16 @@ const MANIFEST_ROOTS = [ * external state read at request time, never RI-committed source or * RI-committed JSON, so it was never a candidate for this allowlist either * (see `reference-implementation/server/version-disposition.ts`'s own module - * doc comment). Left as an explicit empty Map (not deleted) so a future - * legitimate RI-owned sibling registry has an obvious, documented place to - * register — adding an entry here is a real security decision, same bar as - * `SANCTIONED_GENERIC_DATA_READ_CALL_SITES` below. + * doc comment). The canonical Collection Profile observation schema is + * registered for its exact runtime loader. It describes protocol shapes, not + * provider policy; the content check still rejects connector identity in it. + * Additional entries require the same review as the generic read sites below. */ -const SANCTIONED_POLICY_RESOURCES: ReadonlyMap> = new Map([]); +const SANCTIONED_POLICY_RESOURCES: ReadonlyMap> = new Map([ + ["reference-implementation/runtime/observations.ts", new Set([ + "reference-implementation/runtime/observation.schema.json", + ])], +]); const POLYFILL_MANIFEST_READ_SITE = 'reference-implementation/server/polyfill-manifest-reconcile.ts::readManifestJson::readFile(path, "utf8")'; diff --git a/reference-implementation/test/ref-run-observations-route.test.ts b/reference-implementation/test/ref-run-observations-route.test.ts new file mode 100644 index 000000000..dc7b05b6c --- /dev/null +++ b/reference-implementation/test/ref-run-observations-route.test.ts @@ -0,0 +1,190 @@ +// Copyright The PDP-Connect Contributors +// SPDX-License-Identifier: Apache-2.0 + +// Adapter tests for the owner-only run observations route +// `GET /_ref/runs/:runId/observations` (server/routes/ref-run-observations.ts). +// The route is mounted into a fake Express-like app, as ref-run-status-route +// does. The store is injected, so these tests cover the route's gating and +// namespace behavior, not the store's persistence. + +import assert from "node:assert/strict"; +import test from "node:test"; +import { mountRefRunObservations } from "../server/routes/ref-run-observations.ts"; +import type { + ConnectorObservationRunKey, + ConnectorObservationRunRecord, + ConnectorObservationRunStore, +} from "../server/stores/connector-observation-store.ts"; + +const ROUTE = "GET /_ref/runs/:runId/observations"; +const CONNECTOR_ID = "https://registry.pdpp.dev/connectors/observations-route"; +const INSTANCE_ID = "instance_owner_1"; +const RUN_ID = "run_observations_route"; + +type Session = "owner" | "client" | "anonymous"; + +interface Captured { + body: Record | null; + headers: Record; + status: number; +} + +function fakeRecord(): ConnectorObservationRunRecord { + return { + connectorId: CONNECTOR_ID, + connectorInstanceId: INSTANCE_ID, + diagnosis: { attribution: "none", basis: [], category: "unknown", contributing: [] }, + recordedAt: "2026-11-02T00:00:00.000Z", + runId: RUN_ID, + snapshot: { defects: [], facts: [], truncated: false }, + }; +} + +function fakeStore(record: ConnectorObservationRunRecord | null): ConnectorObservationRunStore { + const calls: unknown[] = []; + return { + get(key: ConnectorObservationRunKey) { + calls.push(key); + if ( + record && + key.connectorId === record.connectorId && + key.connectorInstanceId === record.connectorInstanceId && + key.runId === record.runId + ) { + return Promise.resolve(record); + } + return Promise.resolve(null); + }, + list() { + return Promise.resolve([]); + }, + put() { + return Promise.reject(new Error("the diagnostic route never writes")); + }, + } as unknown as ConnectorObservationRunStore; +} + +// Runs the route the way the HTTP stack would: session middleware first, then +// the handler only when the middleware calls next. +async function invoke( + session: Session, + query: Record, + store: ConnectorObservationRunStore, + runId = RUN_ID +): Promise { + type Handler = (req: unknown, res: unknown, next?: () => void) => unknown; + const routes: { middleware: Handler[]; handler: Handler | undefined } = { handler: undefined, middleware: [] }; + const app = { + get(path: string, ...handlers: Handler[]) { + assert.equal(`GET ${path}`, ROUTE); + routes.middleware = handlers.slice(0, -1); + routes.handler = handlers.at(-1); + return app; + }, + }; + mountRefRunObservations(app as never, { + handleError: (_res, err) => { + throw err; + }, + pdppError: (res, status, code, message, param) => { + (res as { status: (n: number) => { json: (b: unknown) => void } }).status(status).json({ + error: { code, message, ...(param ? { param } : {}) }, + }); + }, + requireOwnerSession: (( + _req: unknown, + res: { status: (n: number) => { json: (b: unknown) => void } }, + next: () => void + ) => { + if (session === "owner") { + next(); + return; + } + res + .status(session === "client" ? 403 : 401) + .json({ error: { code: session === "client" ? "forbidden" : "unauthorized" } }); + }) as never, + store, + }); + + const captured: Captured = { body: null, headers: {}, status: 200 }; + const response = { + json(body: unknown) { + captured.body = body as Record; + return response; + }, + setHeader(name: string, value: string) { + captured.headers[name.toLowerCase()] = value; + return response; + }, + status(code: number) { + captured.status = code; + return response; + }, + }; + const req = { params: { runId }, query }; + let nextCalled = false; + const runMiddleware = (fn: Handler) => + fn(req, response, () => { + nextCalled = true; + }); + for (const middleware of routes.middleware) { + nextCalled = false; + runMiddleware(middleware); + if (!nextCalled) { + return captured; + } + } + await routes.handler?.(req, response); + return captured; +} + +const OWNER_QUERY = { connector_id: CONNECTOR_ID, connector_instance_id: INSTANCE_ID }; + +test("an anonymous request is refused by the owner session gate", async () => { + const result = await invoke("anonymous", OWNER_QUERY, fakeStore(fakeRecord())); + assert.equal(result.status, 401); + assert.equal((result.body?.error as { code?: string } | undefined)?.code, "unauthorized"); + assert.notEqual(result.body?.object, "run_observations"); +}); + +test("a client credential is refused by the owner session gate", async () => { + const result = await invoke("client", OWNER_QUERY, fakeStore(fakeRecord())); + assert.equal(result.status, 403); + assert.equal((result.body?.error as { code?: string } | undefined)?.code, "forbidden"); + assert.notEqual(result.body?.object, "run_observations"); +}); + +test("an owner session reads the stored record", async () => { + const result = await invoke("owner", OWNER_QUERY, fakeStore(fakeRecord())); + assert.equal(result.status, 200); + assert.equal(result.headers["cache-control"], "no-store", "private diagnostic facts must not enter an HTTP cache"); + assert.equal(result.body?.object, "run_observations"); + assert.equal(result.body?.run_id, RUN_ID); + assert.deepEqual(result.body?.snapshot, { defects: [], facts: [], truncated: false }); +}); + +test("a run that exists under another connector or instance is not found", async () => { + const store = fakeStore(fakeRecord()); + const otherConnector = await invoke( + "owner", + { ...OWNER_QUERY, connector_id: "https://registry.pdpp.dev/connectors/other" }, + store + ); + assert.equal(otherConnector.status, 404); + const otherInstance = await invoke("owner", { ...OWNER_QUERY, connector_instance_id: "instance_other" }, store); + assert.equal(otherInstance.status, 404); +}); + +test("an unknown run id is a typed not_found 404", async () => { + const result = await invoke("owner", OWNER_QUERY, fakeStore(null), "run_missing"); + assert.equal(result.status, 404); + assert.equal((result.body?.error as { code?: string } | undefined)?.code, "not_found"); +}); + +test("the route requires both namespace query parameters", async () => { + const missingConnector = await invoke("owner", { connector_instance_id: INSTANCE_ID }, fakeStore(fakeRecord())); + assert.equal(missingConnector.status, 400); + const missingInstance = await invoke("owner", { connector_id: CONNECTOR_ID }, fakeStore(fakeRecord())); + assert.equal(missingInstance.status, 400); +}); diff --git a/reference-implementation/test/ri-zero-connector-knowledge-conformance.test.ts b/reference-implementation/test/ri-zero-connector-knowledge-conformance.test.ts index fc8a1b4d3..9217b0814 100644 --- a/reference-implementation/test/ri-zero-connector-knowledge-conformance.test.ts +++ b/reference-implementation/test/ri-zero-connector-knowledge-conformance.test.ts @@ -69,6 +69,36 @@ import { const __dirname = dirname(fileURLToPath(import.meta.url)); const repoRoot = dirname(dirname(__dirname)); +test("the pinned observation schema has one exact loader and cannot carry connector identity", () => { + const fixtureRoot = mkdtempSync(join(tmpdir(), "ri-protocol-schema-resource-")); + const sourceRel = "reference-implementation/runtime/observations.ts"; + const sourceFile = join(fixtureRoot, sourceRel); + const schemaFile = join(dirname(sourceFile), "observation.schema.json"); + const source = 'import schema from "./observation.schema.json" with { type: "json" };\nexport default schema;\n'; + try { + mkdirSync(dirname(sourceFile), { recursive: true }); + writeFileSync(sourceFile, source); + writeFileSync(schemaFile, JSON.stringify({ enum: ["matched", "absent"] })); + assert.deepEqual(scanFileDataLoads(sourceFile, sourceRel, fixtureRoot), [], + "the protocol schema is a generic resource, not connector-specific policy"); + + writeFileSync(schemaFile, JSON.stringify({ nested: { enum: ["synthetic_provider_identity"] } })); + const injected = scanFileDataLoads(sourceFile, sourceRel, fixtureRoot, new Set(["synthetic_provider_identity"])); + assert.ok(injected.some((violation) => violation.rule === "hardcoded-connector-literal-in-ri-owned-json"), + "sanctioning the schema path must retain the provider-content check"); + + writeFileSync(schemaFile, JSON.stringify({ enum: ["matched"] })); + const otherRel = "reference-implementation/runtime/other-loader.ts"; + const otherFile = join(fixtureRoot, otherRel); + writeFileSync(otherFile, source); + assert.ok(scanFileDataLoads(otherFile, otherRel, fixtureRoot) + .some((violation) => violation.rule === "unsanctioned-policy-resource-path"), + "a different loader does not inherit the schema exception"); + } finally { + rmSync(fixtureRoot, { force: true, recursive: true }); + } +}); + test("manifest-derived connector identity set is non-trivial (sanity check on the derivation itself)", () => { const keys = manifestDerivedConnectorKeys({ repoRoot }); // A hardcoded lower-bound count would itself be exactly the kind of diff --git a/reference-implementation/test/runtime-observations.test.ts b/reference-implementation/test/runtime-observations.test.ts new file mode 100644 index 000000000..5ac1ecbd8 --- /dev/null +++ b/reference-implementation/test/runtime-observations.test.ts @@ -0,0 +1,791 @@ +// Copyright The PDP-Connect Contributors +// SPDX-License-Identifier: Apache-2.0 + +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + type AcceptOutcome, + type CreateObservationRunInput, + createObservationRun, + type DiagnosticDescriptors, + ObservationContractError, + type ObservationLimits, + type ObservationRun, + validateDiagnosticDescriptors, +} from "../runtime/observations.ts"; + +const SECRET = "SECRET-MARKER-7f3a9c"; + +const DESCRIPTORS: DiagnosticDescriptors = validateDiagnosticDescriptors({ + expectations: [{ id: "email_input", step: "sign_in" }], + rules: [{ id: "password_rejected", kind: "auth", step: "sign_in" }], + steps: [{ id: "sign_in" }], +}); + +interface Harness { + readonly run: ObservationRun; + setNow: (ms: number) => void; +} + +function harness( + options: { declared?: boolean; limits?: ObservationLimits; descriptors?: DiagnosticDescriptors } = {} +): Harness { + let clock = 0; + const input: CreateObservationRunInput = { + declared: options.declared ?? true, + descriptors: options.descriptors ?? DESCRIPTORS, + now: () => clock, + ...(options.limits === undefined ? {} : { limits: options.limits }), + }; + return { + run: createObservationRun(input), + setNow(ms: number) { + clock = ms; + }, + }; +} + +function expectation(id: string, final: "matched" | "hidden" = "hidden", step = "sign_in") { + return { + attrs: { + expectation: "email_input", + final, + states_seen: ["absent", final], + }, + evidence_ref: "trace:step-3", + fact: "element_expectation", + id, + step, + type: "OBSERVATION", + }; +} + +function authFailure(id: string, extra: Record = {}) { + return { + attrs: { kind: "auth_failure", ...extra }, + fact: "provider_message", + id, + type: "OBSERVATION", + }; +} + +function waitExpired(id: string) { + return { + attrs: { awaited: "email_input", budget_ms: 5000 }, + fact: "wait_expired", + id, + step: "sign_in", + type: "OBSERVATION", + }; +} + +function rejectedReason(outcome: AcceptOutcome): string { + assert.equal(outcome.status, "rejected"); + return outcome.reason; +} + +// --------------------------------------------------------------------------- +// Descriptors +// --------------------------------------------------------------------------- + +test("absent diagnostic_descriptors normalize to empty arrays", () => { + assert.deepEqual(validateDiagnosticDescriptors(undefined), { + expectations: [], + rules: [], + steps: [], + }); +}); + +test("canonical descriptors normalize to closed typed members", () => { + const normalized = validateDiagnosticDescriptors({ + expectations: [{ id: "email_input", step: "sign_in" }], + rules: [ + { id: "password_rejected", kind: "auth", step: "sign_in" }, + { id: "any_auth", kind: "auth" }, + ], + steps: [{ id: "sign_in" }], + }); + assert.deepEqual(normalized, { + expectations: [{ id: "email_input", step: "sign_in" }], + rules: [ + { id: "password_rejected", kind: "auth", step: "sign_in" }, + { id: "any_auth", kind: "auth" }, + ], + steps: [{ id: "sign_in" }], + }); +}); + +test("descriptor syntax, extra members and rule kind are rejected with a bounded code", () => { + const malformed: unknown[] = [ + { steps: [{ id: "Sign_In" }] }, + { steps: [{ id: "a".repeat(65) }] }, + { steps: [{ id: "sign_in", label: "Sign in" }] }, + { rules: [{ id: "r1", kind: "network" }] }, + { expectations: [{ id: "e1" }] }, + { surprises: [] }, + null, + ]; + for (const value of malformed) { + assert.throws( + () => validateDiagnosticDescriptors(value), + (error: unknown) => error instanceof ObservationContractError && error.code === "descriptor_schema" + ); + } +}); + +test("duplicate descriptor ids and undeclared expectation steps are rejected", () => { + const cases: [unknown, string][] = [ + [{ steps: [{ id: "a" }, { id: "a" }] }, "descriptor_duplicate_id"], + [ + { + expectations: [ + { id: "e", step: "a" }, + { id: "e", step: "a" }, + ], + steps: [{ id: "a" }], + }, + "descriptor_duplicate_id", + ], + [ + { + rules: [ + { id: "r", kind: "auth" }, + { id: "r", kind: "auth" }, + ], + steps: [{ id: "a" }], + }, + "descriptor_duplicate_id", + ], + [{ expectations: [{ id: "e", step: "b" }], steps: [{ id: "a" }] }, "descriptor_undeclared_step"], + ]; + for (const [value, code] of cases) { + assert.throws( + () => validateDiagnosticDescriptors(value), + (error: unknown) => error instanceof ObservationContractError && error.code === code + ); + } +}); + +test("an optional rule step follows the canonical schema without an extra reference restriction", () => { + assert.deepEqual( + validateDiagnosticDescriptors({ + rules: [{ id: "r", kind: "auth", step: "b" }], + }).rules, + [{ id: "r", kind: "auth", step: "b" }] + ); +}); + +test("an invalid observation cannot reuse its earlier id for an accepted fact", () => { + const { run } = harness(); + assert.equal(rejectedReason(run.accept({ ...authFailure("used"), attrs: { kind: "invalid" } })), "invalid"); + assert.equal(rejectedReason(run.accept(authFailure("used"))), "duplicate_id"); + assert.equal(run.snapshot().facts.length, 0); +}); + +test("a raw line is measured in UTF-8 bytes before parse; a line of 2049 bytes is oversize", () => { + const line = (size: number) => { + const base = JSON.stringify(waitExpired("b1")); + return base + " ".repeat(size - base.length); + }; + const { run } = harness(); + assert.equal(run.acceptLine(line(2048)).status, "accepted"); + const over = harness(); + assert.equal(rejectedReason(over.run.acceptLine(line(2049))), "oversize"); + assert.equal(over.run.snapshot().truncated, true); +}); + +test("a multi-byte character counts by its UTF-8 length, not its UTF-16 length", () => { + const { run } = harness(); + const base = JSON.stringify(waitExpired("b2")); + // 2047 characters, one of them a three-byte euro sign, make 2049 bytes. + const padded = `${base}${" ".repeat(2046 - base.length)}\u20ac`; + assert.equal(padded.length < 2048, true); + assert.equal(Buffer.byteLength(padded, "utf8"), 2049); + assert.equal(rejectedReason(run.acceptLine(padded)), "oversize"); +}); + +test("a raw line that is not JSON is invalid and is not echoed", () => { + const { run } = harness(); + assert.equal(rejectedReason(run.acceptLine(`{"id": "${SECRET}"`)), "invalid"); + assert.equal(JSON.stringify(run.snapshot()).includes(SECRET), false); +}); + +test("a raw line from an undeclared connector is rejected as undeclared_capability", () => { + const { run } = harness({ declared: false }); + assert.equal(rejectedReason(run.acceptLine(JSON.stringify(waitExpired("u1")))), "undeclared_capability"); +}); + +test("hasEmittedId reports rejected ids as well as accepted ones", () => { + const { run } = harness(); + run.accept({ + ...expectation("h1"), + attrs: { ...expectation("h1").attrs, final: "matched", states_seen: ["absent"] }, + }); + assert.equal(run.hasEmittedId("h1"), true); + assert.equal(run.hasEmittedId("never"), false); +}); + +test("a descriptor error never echoes the rejected value", () => { + assert.throws( + () => validateDiagnosticDescriptors({ steps: [{ id: SECRET }] }), + (error: unknown) => { + assert.ok(error instanceof ObservationContractError); + assert.equal(String(error.message).includes(SECRET), false); + return true; + } + ); +}); + +// --------------------------------------------------------------------------- +// Accept: the three connector fact types and schema grammar +// --------------------------------------------------------------------------- + +test("all three connector fact types are accepted and visible in the snapshot", () => { + const { run } = harness(); + assert.deepEqual(run.accept(expectation("o1")), { + id: "o1", + retired: [], + status: "accepted", + }); + assert.deepEqual(run.accept(waitExpired("o2")), { + id: "o2", + retired: [], + status: "accepted", + }); + assert.deepEqual(run.accept(authFailure("o3", { rule: "password_rejected" })), { + id: "o3", + retired: [], + status: "accepted", + }); + + const snap = run.snapshot(); + assert.deepEqual( + snap.facts.map((fact) => [fact.id, fact.fact, fact.retired]), + [ + ["o1", "element_expectation", false], + ["o2", "wait_expired", false], + ["o3", "provider_message", false], + ] + ); + assert.equal(snap.truncated, false); + assert.deepEqual(snap.defects, []); +}); + +test("schema forbids extra members, provider text, URL evidence and bad id grammar", () => { + const { run } = harness(); + const invalid: unknown[] = [ + { ...expectation("x1"), password: SECRET }, + { + ...expectation("x2"), + attrs: { ...expectation("x2").attrs, message: SECRET }, + }, + authFailure("x3", { detail: SECRET }), + { ...expectation("x4"), evidence_ref: "https://example.test/trace" }, + { ...expectation("x5"), evidence_ref: "trace step 3" }, + { ...expectation("Bad Id!") }, + { + ...waitExpired("x6"), + attrs: { awaited: "email_input", budget_ms: 86_400_001 }, + }, + { + ...expectation("x7"), + resolves: Array.from({ length: 17 }, (_, i) => `r${i}`), + }, + { ...expectation("x8"), fact: "http_response" }, + ]; + for (const value of invalid) { + assert.equal(rejectedReason(run.accept(value)), "invalid"); + } + assert.equal(run.snapshot().facts.length, 0); +}); + +test("final must be a member of states_seen", () => { + const { run } = harness(); + const value = { + attrs: { + expectation: "email_input", + final: "hidden", + states_seen: ["absent"], + }, + fact: "element_expectation", + id: "f1", + step: "sign_in", + type: "OBSERVATION", + }; + assert.equal(rejectedReason(run.accept(value)), "invalid"); + assert.deepEqual(run.snapshot().facts, []); +}); + +test("runtime-only fact types are rejected as runtime_fact_type and never stored", () => { + const { run } = harness(); + for (const fact of ["credential_submission", "rule_match", "connector_defect"]) { + const value = { + attrs: { outcome: "rejected" }, + fact, + id: `forged-${fact}`, + type: "OBSERVATION", + }; + assert.equal(rejectedReason(run.accept(value)), "runtime_fact_type"); + } + const snap = run.snapshot(); + assert.deepEqual(snap.facts, []); + assert.deepEqual( + snap.defects.map((defect) => [defect.class, defect.class === "protocol_violation" ? defect.reason : null]), + [["protocol_violation", "runtime_fact_type"]] + ); +}); + +test("an undeclared OBSERVATION capability rejects every fact", () => { + const { run } = harness({ declared: false }); + assert.equal(rejectedReason(run.accept(expectation("o1"))), "undeclared_capability"); + assert.equal(rejectedReason(run.accept(authFailure("o2"))), "undeclared_capability"); + const snap = run.snapshot(); + assert.deepEqual(snap.facts, []); + assert.equal(snap.truncated, false); + assert.deepEqual( + snap.defects.map((defect) => (defect.class === "protocol_violation" ? defect.reason : null)), + ["undeclared_capability"] + ); +}); + +test("facts naming undeclared step, expectation or rule ids are accepted but support nothing", () => { + const { run } = harness(); + run.accept(expectation("u1", "hidden", "other_step")); + run.accept(authFailure("u2", { rule: "unknown_rule" })); + run.accept({ + attrs: { + expectation: "not_declared", + final: "absent", + states_seen: ["absent"], + }, + fact: "element_expectation", + id: "u3", + step: "sign_in", + type: "OBSERVATION", + }); + const snap = run.snapshot(); + assert.deepEqual( + snap.facts.map((fact) => fact.id), + ["u1", "u2", "u3"] + ); + assert.deepEqual(run.diagnose(["u1", "u2", "u3"]), { + attribution: "none", + basis: [], + category: "unknown", + contributing: [], + }); +}); + +test("duplicate accepted ids are rejected as duplicate_id and the first fact is kept", () => { + const { run } = harness(); + assert.equal(run.accept(expectation("o1", "hidden")).status, "accepted"); + assert.equal(rejectedReason(run.accept(authFailure("o1"))), "duplicate_id"); + const snap = run.snapshot(); + assert.equal(snap.facts.length, 1); + assert.equal(snap.facts[0]?.fact, "element_expectation"); +}); + +// --------------------------------------------------------------------------- +// Retirement and runtime records +// --------------------------------------------------------------------------- + +test("resolves retires an earlier connector fact, which stays visible", () => { + const { run } = harness(); + run.accept(expectation("o1")); + assert.deepEqual(run.accept({ ...waitExpired("o2"), resolves: ["o1"] }), { + id: "o2", + retired: ["o1"], + status: "accepted", + }); + const snap = run.snapshot(); + assert.deepEqual( + snap.facts.map((fact) => [fact.id, fact.retired]), + [ + ["o1", true], + ["o2", false], + ] + ); + assert.deepEqual(run.diagnose(["o1"]), { + attribution: "none", + basis: [], + category: "unknown", + contributing: [], + }); +}); + +test("unknown resolves ids are a no-op and rejected facts retire nothing", () => { + const { run } = harness(); + run.accept(expectation("o1")); + assert.deepEqual(run.accept({ ...waitExpired("o2"), resolves: ["never_seen"] }), { + id: "o2", + retired: [], + status: "accepted", + }); + assert.equal(rejectedReason(run.accept({ ...waitExpired("o3"), extra: true, resolves: ["o1"] })), "invalid"); + assert.equal(run.snapshot().facts[0]?.retired, false); +}); + +test("resolves cannot name or retire a runtime record", () => { + const { run } = harness(); + run.accept(authFailure("o1")); + run.accept(expectation("o2")); + // Runtime record ids contain ":", which the connector id grammar forbids, so the fact is invalid. + assert.equal( + rejectedReason( + run.accept({ + ...waitExpired("o3"), + resolves: ["runtime:connector_defect:protocol_violation:invalid"], + }) + ), + "invalid" + ); + assert.equal(rejectedReason(run.accept({ ...waitExpired("o4"), resolves: ["runtime:rule_match:x"] })), "invalid"); + const snap = run.snapshot(); + assert.deepEqual( + snap.facts.map((fact) => [fact.id, fact.retired]), + [ + ["o1", false], + ["o2", false], + ] + ); + assert.deepEqual(snap.defects, [ + { + class: "protocol_violation", + count: 2, + id: "runtime:connector_defect:protocol_violation:invalid", + reason: "invalid", + }, + ]); + assert.equal(run.diagnose(["o1", "o2"]).category, "auth_rejected"); +}); + +// --------------------------------------------------------------------------- +// Secrecy, aliasing and immutability +// --------------------------------------------------------------------------- + +test("a secret marker in rejected input never appears in defects, snapshot or outcomes", () => { + const { run } = harness(); + const outcomes = [ + run.accept({ ...expectation("s1"), password: SECRET }), + run.accept({ + ...authFailure("s2"), + attrs: { kind: "auth_failure", note: SECRET }, + }), + run.accept({ fact: "wait_expired", id: SECRET, type: "OBSERVATION" }), + run.accept({ + attrs: { handle: SECRET }, + fact: "rule_match", + id: "s3", + type: "OBSERVATION", + }), + run.accept({ ...expectation("s4"), padding: "x".repeat(4000) + SECRET }), + ]; + assert.deepEqual( + outcomes.map((outcome) => (outcome.status === "rejected" ? outcome.reason : outcome.status)), + ["invalid", "invalid", "invalid", "runtime_fact_type", "oversize"] + ); + assert.equal(JSON.stringify(outcomes).includes(SECRET), false); + assert.equal(JSON.stringify(run.snapshot()).includes(SECRET), false); +}); + +test("accepted facts are copies: caller mutation does not reach the run, and snapshots are frozen", () => { + const { run } = harness(); + const input = expectation("o1"); + run.accept(input); + input.attrs.final = "matched"; + input.attrs.states_seen.push("matched"); + const snap = run.snapshot(); + const [stored] = snap.facts; + assert.ok(stored); + assert.equal(stored.fact === "element_expectation" ? stored.attrs.final : null, "hidden"); + assert.equal(Object.isFrozen(stored), true); + assert.throws(() => { + (stored as { id: string }).id = "changed"; + }, TypeError); + assert.equal(run.diagnose(["o1"]).category, "expectation_mismatch"); +}); + +// --------------------------------------------------------------------------- +// Limits: bytes, count, rate, defect capacity +// --------------------------------------------------------------------------- + +test("a 2048-byte raw line is accepted and no oversize truncation is recorded", () => { + const base = JSON.stringify(waitExpired("b1")); + const { run } = harness(); + assert.equal(run.acceptLine(base + " ".repeat(2048 - base.length)).status, "accepted"); + assert.equal(run.snapshot().truncated, false); +}); + +test("the byte bound is measured before validation when no serialized size is supplied", () => { + const { run } = harness(); + const padded = { ...expectation("b3"), padding: "x".repeat(3000) }; + assert.equal(rejectedReason(run.accept(padded)), "oversize"); +}); + +test("64 facts are accepted with a 64-fact limit and the 65th is count_exceeded", () => { + const { run } = harness({ limits: { maxFacts: 64, maxPerWindow: 1000 } }); + for (let i = 0; i < 64; i += 1) { + assert.equal(run.accept(waitExpired(`c${i}`)).status, "accepted"); + } + assert.equal(rejectedReason(run.accept(waitExpired("c64"))), "count_exceeded"); + const snap = run.snapshot(); + assert.equal(snap.facts.length, 64); + assert.equal(snap.truncated, true); +}); + +test("configured limits cannot fall below the profile minimums", () => { + for (const limits of [{ maxFacts: 63 }, { maxFactBytes: 2047 }, { maxFacts: 0 }, { maxPerWindow: 1.5 }]) { + assert.throws( + () => harness({ limits }), + (error: unknown) => error instanceof ObservationContractError && error.code === "limits_invalid" + ); + } +}); + +test("the default capacity is 256 facts on a single clock tick", () => { + const { run } = harness(); + for (let i = 0; i < 256; i += 1) { + assert.equal(run.accept(waitExpired(`r${i}`)).status, "accepted"); + } + assert.equal(rejectedReason(run.accept(waitExpired("r256"))), "count_exceeded"); + assert.equal(run.snapshot().facts.length, 256); +}); + +test("the default rate is 256 messages per 1000 ms window on an injected clock", () => { + const { run, setNow } = harness({ limits: { maxFacts: 1000 } }); + for (let i = 0; i < 256; i += 1) { + assert.equal(run.accept(waitExpired(`t${i}`)).status, "accepted"); + } + assert.equal(rejectedReason(run.accept(waitExpired("t256"))), "count_exceeded"); + setNow(999); + assert.equal(rejectedReason(run.accept(waitExpired("t257"))), "count_exceeded"); + setNow(1000); + assert.equal(run.accept(waitExpired("t258")).status, "accepted"); + assert.equal(run.snapshot().truncated, true); +}); + +test("a flood of violations keeps defect records bounded and does not erase accepted evidence", () => { + const { run } = harness({ + limits: { maxFacts: 64, maxPerWindow: 1_000_000 }, + }); + assert.equal(run.accept(authFailure("keep")).status, "accepted"); + for (let i = 0; i < 10_000; i += 1) { + run.accept({ fact: "nope", id: `bad-${i}`, type: "OBSERVATION" }); + run.accept({ ...waitExpired(`flood-${i}`), padding: "x".repeat(4096) }); + run.accept({ + fact: "connector_defect", + id: `dupe-${i}`, + type: "OBSERVATION", + }); + } + const snap = run.snapshot(); + assert.ok(snap.defects.length <= 7, `defect records bounded, got ${snap.defects.length}`); + assert.equal(snap.truncated, true); + assert.ok(snap.facts.some((fact) => fact.id === "keep" && fact.retired === false)); + assert.equal(run.diagnose(["keep"]).category, "auth_rejected"); +}); + +test("violation defects aggregate by reason with counts and no connector text", () => { + const { run } = harness(); + for (let i = 0; i < 5; i += 1) { + run.accept({ + attrs: { note: SECRET }, + fact: "invalid_fact", + id: `x${i}`, + type: "OBSERVATION", + }); + } + const snap = run.snapshot(); + assert.deepEqual(snap.defects, [ + { + class: "protocol_violation", + count: 5, + id: "runtime:connector_defect:protocol_violation:invalid", + reason: "invalid", + }, + ]); + assert.equal(JSON.stringify(snap).includes(SECRET), false); +}); + +// --------------------------------------------------------------------------- +// Diagnosis +// --------------------------------------------------------------------------- + +test("an auth_failure cited in the basis diagnoses auth_rejected attributed to the connector", () => { + const { run } = harness(); + run.accept(authFailure("a1")); + assert.deepEqual(run.diagnose(["a1"]), { + attribution: "connector", + basis: ["a1"], + category: "auth_rejected", + contributing: [], + }); +}); + +test("auth_rejected outranks expectation_mismatch and the mismatch is listed as contributing", () => { + const { run } = harness(); + run.accept(expectation("e1", "hidden")); + run.accept(authFailure("a1", { rule: "password_rejected" })); + assert.deepEqual(run.diagnose(["e1", "a1"]), { + attribution: "connector", + basis: ["a1"], + category: "auth_rejected", + contributing: ["e1"], + }); +}); + +test("a declared expectation that did not match diagnoses expectation_mismatch", () => { + const { run } = harness(); + run.accept(expectation("e1", "hidden")); + assert.deepEqual(run.diagnose(["e1"]), { + attribution: "connector", + basis: ["e1"], + category: "expectation_mismatch", + contributing: [], + }); +}); + +test("matched expectations, wait_expired and uncited facts never support a cause", () => { + const { run } = harness(); + run.accept(expectation("m1", "matched")); + run.accept(waitExpired("w1")); + run.accept(authFailure("a1")); + assert.equal(run.diagnose(["m1", "w1"]).category, "unknown"); + assert.equal(run.diagnose([]).category, "unknown"); + assert.equal(run.diagnose().category, "unknown"); + assert.equal(run.diagnose(["unknown_id"]).category, "unknown"); +}); + +test("an auth_failure with kind other or a provider message under undeclared rule supports nothing", () => { + const { run } = harness(); + run.accept({ + attrs: { kind: "other" }, + fact: "provider_message", + id: "a1", + type: "OBSERVATION", + }); + run.accept(authFailure("a2", { rule: "undeclared" })); + run.accept({ + attrs: { kind: "auth_failure" }, + fact: "provider_message", + id: "a3", + step: "undeclared_step", + type: "OBSERVATION", + }); + assert.equal(run.diagnose(["a1", "a2", "a3"]).category, "unknown"); +}); + +test("ties break by record order and basis is listed in that order", () => { + const { run } = harness(); + run.accept(authFailure("a2")); + run.accept(authFailure("a1")); + assert.deepEqual(run.diagnose(["a1", "a2"]).basis, ["a2", "a1"]); + assert.deepEqual(run.diagnose(["a1", "a2"]).basis, run.diagnose(["a2", "a1"]).basis); +}); + +test("a retired fact cannot become the cause", () => { + const { run } = harness(); + run.accept(authFailure("a1")); + run.accept({ ...waitExpired("w1"), resolves: ["a1"] }); + assert.equal(run.diagnose(["a1"]).category, "unknown"); +}); + +test("diagnosis output carries no action authority", () => { + const { run } = harness(); + run.accept(authFailure("a1")); + const diagnosis = run.diagnose(["a1"]); + assert.deepEqual( + Object.keys(diagnosis).sort((a, b) => (a < b ? -1 : Number(a > b))), + ["attribution", "basis", "category", "contributing"] + ); + assert.equal(run.gateRecoveryHint("refresh_credentials", "skip_result").result, "unsupported"); +}); + +// --------------------------------------------------------------------------- +// Recovery-hint gate +// --------------------------------------------------------------------------- + +test("not_retriable and unknown hold; other hints are unsupported", () => { + const { run } = harness(); + assert.deepEqual(run.gateRecoveryHint("not_retriable", "skip_result"), { + result: "authorized", + }); + assert.deepEqual(run.gateRecoveryHint("unknown", "skip_result"), { result: "authorized" }); + for (const hint of [ + "refresh_credentials", + "manual_action_required", + "update_selector", + "retry_on_connector_upgrade", + "upstream_unblock", + "retry_by_runtime", + ]) { + assert.deepEqual(run.gateRecoveryHint(hint, "skip_result"), { result: "unsupported" }); + } +}); + +test("an unsupported hint from a declarer adds one bounded hint_unsupported defect and never echoes the action", () => { + const { run } = harness(); + run.gateRecoveryHint("refresh_credentials", "skip_result"); + run.gateRecoveryHint(`SECRET-ACTION-${SECRET}`, "skip_result"); + assert.deepEqual(run.snapshot().defects, [ + { + class: "hint_unsupported", + count: 2, + id: "runtime:connector_defect:hint_unsupported", + }, + ]); + assert.equal(JSON.stringify(run.snapshot()).includes(SECRET), false); +}); + +test("a non-declarer's unsupported hint adds no defect", () => { + const { run } = harness({ declared: false }); + assert.deepEqual(run.gateRecoveryHint("manual_action_required", "skip_result"), { + result: "unsupported", + }); + assert.deepEqual(run.snapshot().defects, []); +}); + +test("a truncated record blocks even a holding hint", () => { + const { run } = harness({ limits: { maxFacts: 64, maxPerWindow: 1000 } }); + for (let i = 0; i < 65; i += 1) { + run.accept(waitExpired(`t${i}`)); + } + assert.equal(run.snapshot().truncated, true); + assert.deepEqual(run.gateRecoveryHint("not_retriable", "skip_result"), { + reason: "truncated", + result: "blocked", + }); + assert.deepEqual(run.gateRecoveryHint("unknown", "skip_result"), { + reason: "truncated", + result: "blocked", + }); + assert.equal( + run.snapshot().defects.find((d) => d.class === "hint_unsupported")?.count, + 2, + "Section 5.11 records unsupported hints even when truncation caused the denial" + ); +}); + +test("connector auth facts never unlock a recovery hint", () => { + const { run } = harness(); + run.accept(authFailure("a1", { rule: "password_rejected" })); + assert.deepEqual(run.diagnose(["a1"]).category, "auth_rejected"); + assert.deepEqual(run.gateRecoveryHint("refresh_credentials", "skip_result"), { + result: "unsupported", + }); +}); + +test("an unauthorized connector hint is recorded once per origin and action as a bounded suggestion", () => { + const { run } = harness(); + assert.equal(run.snapshot().connector_suggestions, undefined); + run.gateRecoveryHint("refresh_credentials", "done_error"); + run.gateRecoveryHint("refresh_credentials", "done_error"); + run.gateRecoveryHint("manual_action_required", "skip_result"); + run.gateRecoveryHint(`SECRET-ACTION-${SECRET}`, "skip_result"); + assert.deepEqual(run.snapshot().connector_suggestions, [ + { action: "refresh_credentials", attribution: "connector", origin: "done_error" }, + { action: "manual_action_required", attribution: "connector", origin: "skip_result" }, + ]); + assert.equal(JSON.stringify(run.snapshot()).includes(SECRET), false); +}); From 763df08b56385f1e2e483080ab4d34dffd706d9b Mon Sep 17 00:00:00 2001 From: Tim Nunamaker Date: Thu, 5 Nov 2026 13:41:40 -0600 Subject: [PATCH 2/9] fix(runtime): keep a failed DONE whose basis cites an id past the full id ledger The emitted-id ledger stops at the fact limit. A connector that emitted more observations than the ledger holds could cite an id the ledger had refused, and the runtime then rejected the whole DONE as citing an id it did not emit. That turned the connector's own terminal outcome into a protocol failure. After the ledger overflows, a cited id missing from it can no longer be shown false. The validator now drops that id from the stored basis and keeps the DONE. Before overflow, an unknown id is still rejected. Assisted-by: AI Signed-off-by: Tim Nunamaker --- .../runtime/done-validators.ts | 20 +++++++++- reference-implementation/runtime/index.ts | 1 + .../runtime/observations.ts | 9 +++++ .../done-error-normalization-oracle.test.ts | 2 +- .../test/runtime-observations.test.ts | 38 +++++++++++++++++++ 5 files changed, 67 insertions(+), 3 deletions(-) diff --git a/reference-implementation/runtime/done-validators.ts b/reference-implementation/runtime/done-validators.ts index 742b9cc79..5a99ac18c 100644 --- a/reference-implementation/runtime/done-validators.ts +++ b/reference-implementation/runtime/done-validators.ts @@ -93,12 +93,19 @@ export interface DoneErrorContract { readonly observationDeclared: boolean; /** True when the connector emitted this OBSERVATION id earlier in the run. */ readonly hasEmittedId: (id: string) => boolean; + /** + * True when the runtime's bounded id ledger refused ids. A cited id missing + * from the ledger may then still have been emitted, so it cannot be proven + * false. + */ + readonly idLedgerOverflowed: boolean; } const LEGACY_DONE_ERROR_CONTRACT: DoneErrorContract = { strict: false, observationDeclared: false, hasEmittedId: () => false, + idLedgerOverflowed: false, }; export function validateDoneError( @@ -163,6 +170,9 @@ export function validateDoneError( * Section 5.8: `basis` is OPTIONAL, needs a declared OBSERVATION capability, * and cites only ids the connector emitted in this run. Invalid-but-emitted * ids count as emitted. Returns the ids, undefined when absent, or an Error. + * After the id ledger overflowed, an id missing from it is dropped instead of + * rejected: the runtime cannot show it was never emitted, and a basis it cannot + * verify authorizes nothing. */ function validateDoneBasis(basis: unknown, contract: DoneErrorContract): readonly string[] | undefined | Error { if (basis === undefined) { @@ -174,12 +184,18 @@ function validateDoneBasis(basis: unknown, contract: DoneErrorContract): readonl if (!Array.isArray(basis)) { return new Error("Connector emitted invalid DONE.error.basis: expected array of observation ids"); } + const verified: string[] = []; for (const id of basis) { - if (!(isObservationIdSyntax(id) && contract.hasEmittedId(id))) { + if (!isObservationIdSyntax(id)) { + return new Error("Connector emitted DONE.error.basis citing an OBSERVATION id it did not emit"); + } + if (contract.hasEmittedId(id)) { + verified.push(id); + } else if (!contract.idLedgerOverflowed) { return new Error("Connector emitted DONE.error.basis citing an OBSERVATION id it did not emit"); } } // The emitted-id ledger bounds unique citations. Duplicate citations add no // diagnosis authority and need not expand the stored record. - return [...new Set(basis)] as string[]; + return [...new Set(verified)]; } diff --git a/reference-implementation/runtime/index.ts b/reference-implementation/runtime/index.ts index dd2531ef6..456cd1cfe 100644 --- a/reference-implementation/runtime/index.ts +++ b/reference-implementation/runtime/index.ts @@ -5635,6 +5635,7 @@ export async function runConnector(opts: RuntimeRunConnectorOptions): Promise observationRun.hasEmittedId(id), + idLedgerOverflowed: observationRun.idLedgerOverflowed(), }); if (validatedDoneError instanceof Error) { throw invalidDoneEnvelope(validatedDoneError, msg); diff --git a/reference-implementation/runtime/observations.ts b/reference-implementation/runtime/observations.ts index 708fb4076..a1066db50 100644 --- a/reference-implementation/runtime/observations.ts +++ b/reference-implementation/runtime/observations.ts @@ -198,6 +198,12 @@ export interface ObservationRun { * fact was accepted or rejected. The ledger is bounded by the fact limit. */ hasEmittedId: (id: string) => boolean; + /** + * True once an id was refused because the ledger was full. Later ids may + * have been emitted without being recorded, so absence from the ledger no + * longer proves the connector never emitted an id. + */ + idLedgerOverflowed: () => boolean; snapshot: () => ObservationSnapshot; } @@ -421,6 +427,7 @@ export function createObservationRun(input: CreateObservationRunInput): Observat const facts: StoredFact[] = []; const factsById = new Map(); const observedIds = new Set(); + let idLedgerOverflowed = false; const defects = new Map(); // Keyed by origin and action. The key space is closed (2 origins x 8 // actions), so the record is bounded without a cap. @@ -512,6 +519,7 @@ export function createObservationRun(input: CreateObservationRunInput): Observat return reject("duplicate_id", false); } if (observedIds.size >= limits.maxFacts) { + idLedgerOverflowed = true; return reject("count_exceeded", true); } observedIds.add(value.id); @@ -636,6 +644,7 @@ export function createObservationRun(input: CreateObservationRunInput): Observat diagnose, gateRecoveryHint, hasEmittedId, + idLedgerOverflowed: () => idLedgerOverflowed, snapshot, }; } diff --git a/reference-implementation/test/done-error-normalization-oracle.test.ts b/reference-implementation/test/done-error-normalization-oracle.test.ts index 7b7fd1d4e..662f2245f 100644 --- a/reference-implementation/test/done-error-normalization-oracle.test.ts +++ b/reference-implementation/test/done-error-normalization-oracle.test.ts @@ -25,7 +25,7 @@ test("portable DONE basis can cite more than 64 earlier observations without inv // supported observation count, not a maximum basis length. const basis = Array.from({ length: 65 }, (_, index) => `observation_${index}`); const result = validateDoneError("failed", { message: "failed", retryable: false, basis }, { - strict: true, observationDeclared: true, hasEmittedId: (id) => basis.includes(id), + strict: true, observationDeclared: true, hasEmittedId: (id) => basis.includes(id), idLedgerOverflowed: false, }); assert.ok(!(result instanceof Error), "valid earlier ids must not be rejected by an invented 64-id cap"); assert.deepEqual(result?.basis, basis); diff --git a/reference-implementation/test/runtime-observations.test.ts b/reference-implementation/test/runtime-observations.test.ts index 5ac1ecbd8..ec0c9fd28 100644 --- a/reference-implementation/test/runtime-observations.test.ts +++ b/reference-implementation/test/runtime-observations.test.ts @@ -4,6 +4,7 @@ import assert from "node:assert/strict"; import test from "node:test"; +import { validateDoneError } from "../runtime/done-validators.ts"; import { type AcceptOutcome, type CreateObservationRunInput, @@ -226,6 +227,43 @@ test("hasEmittedId reports rejected ids as well as accepted ones", () => { assert.equal(run.hasEmittedId("never"), false); }); +test("a failed DONE citing an id past the full id ledger is kept without the unverifiable id", () => { + const { run } = harness({ limits: { maxFacts: 64, maxPerWindow: 1000 } }); + for (let i = 0; i < 64; i += 1) { + assert.equal(run.accept(waitExpired(`c${i}`)).status, "accepted"); + } + // The connector did emit this id, but the bounded ledger could not keep it. + assert.equal(rejectedReason(run.accept(waitExpired("c64"))), "count_exceeded"); + const result = validateDoneError( + "failed", + { basis: ["c0", "c64"], message: "stopped", retryable: false }, + { + hasEmittedId: (id) => run.hasEmittedId(id), + idLedgerOverflowed: run.idLedgerOverflowed(), + observationDeclared: true, + strict: true, + } + ); + assert.ok(!(result instanceof Error), "a connector that overflowed the ledger must still get its terminal outcome"); + assert.deepEqual(result?.basis, ["c0"]); +}); + +test("a failed DONE citing an id the connector never emitted is still rejected while the ledger has room", () => { + const { run } = harness(); + run.accept(waitExpired("c0")); + const result = validateDoneError( + "failed", + { basis: ["never"], message: "stopped", retryable: false }, + { + hasEmittedId: (id) => run.hasEmittedId(id), + idLedgerOverflowed: run.idLedgerOverflowed(), + observationDeclared: true, + strict: true, + } + ); + assert.ok(result instanceof Error); +}); + test("a descriptor error never echoes the rejected value", () => { assert.throws( () => validateDiagnosticDescriptors({ steps: [{ id: SECRET }] }), From a7d20c89906ac6d37b4e6176db8056ad7c9840cb Mon Sep 17 00:00:00 2001 From: Tim Nunamaker Date: Thu, 5 Nov 2026 13:43:52 -0600 Subject: [PATCH 3/9] fix(runtime): store the diagnostic record before the terminal run event The runtime wrote the owner diagnostic record after it emitted the terminal run event. A reader who saw the terminal event could find no record, and a crash between the two writes lost the observed facts for good. Both terminal paths now store the record first: the message-failure path and the child-close path. The write is memoized, so the later settle points stay no-ops. A failed store write is still reported as progress and never changes the run outcome. Assisted-by: AI Signed-off-by: Tim Nunamaker --- reference-implementation/runtime/index.ts | 5 ++ .../test/collection-profile-020.test.ts | 54 ++++++++++++++++++- 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/reference-implementation/runtime/index.ts b/reference-implementation/runtime/index.ts index 456cd1cfe..590c07905 100644 --- a/reference-implementation/runtime/index.ts +++ b/reference-implementation/runtime/index.ts @@ -5287,6 +5287,8 @@ export async function runConnector(opts: RuntimeRunConnectorOptions): Promise { const err = caught as RuntimeRunError; finalStatus = "failed"; + // The owner record must exist before any reader can see the terminal event. + await persistObservationDiagnosis(); const failureReason = classifyRuntimeFailure(err); const checkpointSummary = buildCheckpointSummary(); err.run_id = runId; @@ -7228,6 +7230,9 @@ export async function runConnector(opts: RuntimeRunConnectorOptions): Promise { + const store = getDefaultConnectorObservationStore(); + const put = store.put.bind(store); + let terminalSeen: boolean | null = null; + const spy = mock.method(store, "put", async (input: Parameters[0]) => { + // PostgreSQL returns a promise here, SQLite a page; await covers both. + const page = await listSpineEventsPage("run", runId, { limit: 500 }); + terminalSeen = page.events.some((event) => event.event_type === "run.failed"); + return put(input); + }); + try { + await runStub(stub, runId, manifest()); + } finally { + spy.mock.restore(); + } + return terminalSeen; +} + +test("the diagnostic record is stored before the terminal event of a failed DONE", async () => { + const stub = makeStub( + [ + PROVIDER_REJECTED("o1"), + { + error: { basis: ["o1"], code: "credential_rejected", message: "refused", retryable: false }, + records_emitted: 0, + status: "failed", + type: "DONE", + }, + ], + 1 + ); + try { + assert.equal(await terminalEventSeenAtStoreWrite(stub, "run_c020_order_done"), false); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } +}); + +test("the diagnostic record is stored before the terminal event of a protocol violation", async () => { + const done = { records_emitted: 0, status: "succeeded", type: "DONE" }; + const stub = makeStub([PROVIDER_REJECTED("o2"), done, EXPECTATION("o3")], 0); + try { + assert.equal(await terminalEventSeenAtStoreWrite(stub, "run_c020_order_violation"), false); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } +}); + test("uncited, retired and undeclared-rule facts cannot support a cause", async () => { const stub = makeStub( [ From b5aa57e48ed79f7ae038be953ef7d06891e24d52 Mon Sep 17 00:00:00 2001 From: Tim Nunamaker Date: Thu, 5 Nov 2026 13:43:53 -0600 Subject: [PATCH 4/9] fix(runtime): store no diagnosed cause for a run the owner cancelled A failed DONE that cited an authentication fact still produced an auth_rejected cause in the owner record when the owner cancelled the run before it closed. The run ended by the owner's choice, so the record named a cause the connector did not end the run with. The record now ignores the DONE basis when the owner requested the cancellation. The facts stay in the record; only the cause is withheld. Assisted-by: AI Signed-off-by: Tim Nunamaker --- reference-implementation/runtime/index.ts | 3 +- .../test/collection-profile-020.test.ts | 34 ++++++++++++++++++- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/reference-implementation/runtime/index.ts b/reference-implementation/runtime/index.ts index 590c07905..6fb1c32ac 100644 --- a/reference-implementation/runtime/index.ts +++ b/reference-implementation/runtime/index.ts @@ -6561,7 +6561,8 @@ export async function runConnector(opts: RuntimeRunConnectorOptions): Promise, - extraEnv: Record = {} + extraEnv: Record = {}, + runOptions: Record = {} ): Promise { const savedEnv: Record = {}; for (const [key, value] of Object.entries(extraEnv)) { @@ -177,6 +178,7 @@ async function runStub( persistState: false, runId, scope: { streams: [{ name: "items" }] }, + ...runOptions, } as never); return { ok: true, result: result as unknown as Record }; } catch (caught) { @@ -546,6 +548,36 @@ test("the diagnostic record is stored before the terminal event of a protocol vi } }); +test("a cancelled run stores no cause, even when its failed DONE cited an auth fact", async () => { + const stub = makeStub( + [ + PROVIDER_REJECTED("x1"), + { + error: { basis: ["x1"], code: "credential_rejected", message: "refused", retryable: false }, + records_emitted: 0, + status: "failed", + type: "DONE", + }, + ], + 1 + ); + const abort = new AbortController(); + try { + // The owner's cancel lands after DONE was read and before the run closes. + await runStub(stub, "run_c020_cancelled", manifest(), {}, { + cancelSignal: abort.signal, + testOnlyTerminalProcessingFaultInjector: () => abort.abort(), + }); + const record = await persistedRecord("run_c020_cancelled"); + assert.ok(record, "the diagnostic record must be persisted"); + assert.equal(record.diagnosis.category, "unknown"); + assert.equal(record.diagnosis.attribution, "none"); + assert.deepEqual(record.diagnosis.basis, []); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } +}); + test("uncited, retired and undeclared-rule facts cannot support a cause", async () => { const stub = makeStub( [ From 430b036b785a5888ce940be6f581873c37b3bd32 Mon Sep 17 00:00:00 2001 From: Tim Nunamaker Date: Thu, 5 Nov 2026 20:45:56 -0600 Subject: [PATCH 5/9] fix(runtime): keep invalid traffic from using up the OBSERVATION budget An independent audit of the OBSERVATION intake and its diagnostic store found several places where bad input cost a run more than it should, and where stored records were trusted too much. Intake (runtime/observations.ts): - Invalid, oversize and runtime-typed messages no longer use the rate window or the fact limit. Validation runs first, and only a valid fact that a limit refuses takes a rate slot or marks the record truncated. Before, 256 messages with bad `fact` values and distinct ids could fill the id ledger and the rate window, so the connector's next valid facts were discarded as `count_exceeded`. - The ids of discarded messages go in their own bounded set. An accepted fact keeps its id in the fact map, which the fact limit already bounds, so `DONE.error.basis` can always cite an accepted fact. - A line that is not JSON no longer marks the record truncated, however long it is. One 3 KB garbage line used to block every later held hint for the run. The `oversize` reason is still recorded. - The header no longer claims that intake checks declared id references. Sections 3.8 and 5.10.2 say a fact naming an undeclared step, expectation or rule is kept and supports nothing. Diagnosis already enforces that, and a test pins it. DONE basis (runtime/done-validators.ts): - A basis entry that is not a valid OBSERVATION id now gets its own message, not the "did not emit" message. The comment on the overflow drop now says why the drop cannot change a diagnosis. Store (server/stores/connector-observation-store.ts): - `encodeRecord` and `decodeRow` check each stored fact and defect against the runtime shapes, not only that the arrays exist. The check runs after the size bound. - `list` skips a row it cannot read. Before, one corrupt row made the whole list fail. A direct `get` of that row still fails with `record_invalid`, and unparseable JSON now fails the same way. Route (server/routes/ref-run-observations.ts): - A malformed percent-encoded run id, or an over-long identity, is a 400 `invalid_request`. It was an unhandled error. Tests cover each change, with SQLite and PostgreSQL store contracts. Assisted-by: AI Signed-off-by: Tim Nunamaker --- .../runtime/done-validators.ts | 8 +- .../runtime/observations.ts | 125 +++++++++++++----- .../server/routes/ref-run-observations.ts | 33 ++++- .../stores/connector-observation-store.ts | 52 ++++++-- .../test/connector-observation-store.test.ts | 79 +++++++++++ .../test/ref-run-observations-route.test.ts | 16 +++ .../test/runtime-observations.test.ts | 86 ++++++++++-- 7 files changed, 339 insertions(+), 60 deletions(-) diff --git a/reference-implementation/runtime/done-validators.ts b/reference-implementation/runtime/done-validators.ts index 5a99ac18c..becf6fd12 100644 --- a/reference-implementation/runtime/done-validators.ts +++ b/reference-implementation/runtime/done-validators.ts @@ -171,8 +171,10 @@ export function validateDoneError( * and cites only ids the connector emitted in this run. Invalid-but-emitted * ids count as emitted. Returns the ids, undefined when absent, or an Error. * After the id ledger overflowed, an id missing from it is dropped instead of - * rejected: the runtime cannot show it was never emitted, and a basis it cannot - * verify authorizes nothing. + * rejected: the runtime cannot show it was never emitted. The drop cannot + * change the diagnosis. Every accepted fact keeps its id in the ledger, so a + * dropped id names no stored fact and a basis the runtime cannot verify + * authorizes nothing. */ function validateDoneBasis(basis: unknown, contract: DoneErrorContract): readonly string[] | undefined | Error { if (basis === undefined) { @@ -187,7 +189,7 @@ function validateDoneBasis(basis: unknown, contract: DoneErrorContract): readonl const verified: string[] = []; for (const id of basis) { if (!isObservationIdSyntax(id)) { - return new Error("Connector emitted DONE.error.basis citing an OBSERVATION id it did not emit"); + return new Error("Connector emitted DONE.error.basis with a value that is not a valid OBSERVATION id"); } if (contract.hasEmittedId(id)) { verified.push(id); diff --git a/reference-implementation/runtime/observations.ts b/reference-implementation/runtime/observations.ts index a1066db50..a56f21b50 100644 --- a/reference-implementation/runtime/observations.ts +++ b/reference-implementation/runtime/observations.ts @@ -9,9 +9,13 @@ // 5.11 (diagnosis and the recovery-hint gate). The wire grammar is the // canonical JSON Schema `observation.schema.json` beside this file, a byte // copy of data-connectors blob 8d9384d1f0827647e62e1914e8cac81d809034b3. The -// schema cannot express the checks below, so this module adds them: declared -// id references, final-in-states_seen, duplicate ids, the run limits and the -// diagnosis rules. +// schema cannot express the checks below, so this module adds them: +// final-in-states_seen, duplicate ids, the run limits and the diagnosis rules. +// +// A fact that names a step, expectation or rule the manifest does not declare +// is NOT rejected at intake. Sections 3.8 and 5.10.2 say the runtime keeps it, +// shows it to the owner, and lets it support nothing. Diagnosis enforces that +// (findingOf), so an undeclared reference is stored but never a cause. // // The module returns data. It renders no text, retries nothing, prompts for no // credential, and performs no provider or storage I/O. @@ -207,6 +211,52 @@ export interface ObservationRun { snapshot: () => ObservationSnapshot; } +const REJECT_REASONS: ReadonlySet = new Set([ + "runtime_fact_type", + "undeclared_capability", + "invalid", + "duplicate_id", + "oversize", + "count_exceeded", +]); + +function hasOnlyKeys(value: object, keys: readonly string[]): boolean { + return Object.keys(value).every((key) => keys.includes(key)); +} + +/** + * True when `value` has the shape of a stored connector fact: a wire-valid + * fact plus the runtime's `retired` flag, and nothing else. For readers of + * persisted records, which must not trust what they read back. + */ +export function isObservedFact(value: unknown): value is ObservedFact { + if (typeof value !== "object" || value === null || Array.isArray(value)) { + return false; + } + const { retired, ...wire } = value as Record; + return typeof retired === "boolean" && validateMessage(wire); +} + +/** True when `value` has the shape of a runtime defect record this module produces. */ +export function isRuntimeDefect(value: unknown): value is RuntimeDefect { + if (typeof value !== "object" || value === null || Array.isArray(value)) { + return false; + } + const defect = value as Record; + if (!(Number.isSafeInteger(defect.count) && (defect.count as number) >= 1)) { + return false; + } + if (defect.class === "hint_unsupported") { + return hasOnlyKeys(defect, ["id", "class", "count"]) && defect.id === HINT_UNSUPPORTED_ID; + } + return ( + defect.class === "protocol_violation" && + hasOnlyKeys(defect, ["id", "class", "reason", "count"]) && + REJECT_REASONS.has(defect.reason) && + defect.id === `${PROTOCOL_VIOLATION_ID_PREFIX}${String(defect.reason)}` + ); +} + /** Syntax of an OBSERVATION id (schema `observationId`). */ export function isObservationIdSyntax(value: unknown): value is string { return validateObservationId(value) === true; @@ -426,7 +476,10 @@ export function createObservationRun(input: CreateObservationRunInput): Observat const facts: StoredFact[] = []; const factsById = new Map(); - const observedIds = new Set(); + // Ids of discarded messages. An accepted fact's id lives in factsById, which + // the fact limit bounds. Keeping the two apart means invalid traffic cannot + // use up the room that valid facts need. + const discardedIds = new Set(); let idLedgerOverflowed = false; const defects = new Map(); // Keyed by origin and action. The key space is closed (2 origins x 8 @@ -483,7 +536,9 @@ export function createObservationRun(input: CreateObservationRunInput): Observat try { value = JSON.parse(line); } catch { - return bytes > limits.maxFactBytes ? reject("oversize", true) : reject("invalid", false); + // Text that is not JSON is no fact, so no fact was lost: the oversize + // reason is recorded, but the record is not marked truncated. + return bytes > limits.maxFactBytes ? reject("oversize", false) : reject("invalid", false); } return accept(value, bytes); } @@ -496,45 +551,43 @@ export function createObservationRun(input: CreateObservationRunInput): Observat if (bytes === undefined || !Number.isSafeInteger(bytes) || bytes < 0) { return reject("invalid", false); } - // Even a discarded fact has emitted its id (Sections 5.8 and 5.10.2). - const duplicate = rememberEmittedId(value); - if (duplicate) { - return duplicate; + // A repeated id is a duplicate whatever else is wrong with the repeat. Even + // a discarded fact has emitted its id (Sections 5.8 and 5.10.2). + const id = emittedIdOf(value); + if (id !== undefined && hasEmittedId(id)) { + return reject("duplicate_id", false); } - if (bytes > limits.maxFactBytes) { - return reject("oversize", true); + const outcome = bytes > limits.maxFactBytes ? reject("oversize", true) : admitParsed(value); + if (outcome.status === "rejected" && id !== undefined) { + rememberDiscardedId(id); } - return admitParsed(value); + return outcome; } function hasEmittedId(id: string): boolean { - return observedIds.has(id); + return factsById.has(id) || discardedIds.has(id); } - function rememberEmittedId(value: unknown): AcceptOutcome | null { - // An invalid fact still used its id. Bound this ledger independently - // so a stream of malformed messages cannot consume unbounded memory. - if (typeof value === "object" && value !== null && "id" in value && validateObservationId(value.id)) { - if (observedIds.has(value.id)) { - return reject("duplicate_id", false); - } - if (observedIds.size >= limits.maxFacts) { - idLedgerOverflowed = true; - return reject("count_exceeded", true); - } - observedIds.add(value.id); - } - return null; + function emittedIdOf(value: unknown): string | undefined { + return typeof value === "object" && value !== null && "id" in value && validateObservationId(value.id) + ? (value.id as string) + : undefined; } - function admitParsed(value: unknown): AcceptOutcome { - if (!takeRateSlot(now())) { - return reject("count_exceeded", true); - } - if (facts.length >= limits.maxFacts) { - return reject("count_exceeded", true); + function rememberDiscardedId(id: string): void { + if (discardedIds.size >= limits.maxFacts) { + // The id was emitted but cannot be kept. Later citations of ids missing + // from the ledger can no longer be proven false. + idLedgerOverflowed = true; + return; } + discardedIds.add(id); + } + // Validation comes before the rate slot and the fact limit, so invalid + // traffic spends neither. Only a valid fact that a limit refuses marks the + // record truncated. + function admitParsed(value: unknown): AcceptOutcome { if (isRuntimeFactType(value)) { return reject("runtime_fact_type", false); } @@ -548,6 +601,12 @@ export function createObservationRun(input: CreateObservationRunInput): Observat if (fact.fact === "element_expectation" && !fact.attrs.states_seen.includes(fact.attrs.final)) { return reject("invalid", false); } + if (!takeRateSlot(now())) { + return reject("count_exceeded", true); + } + if (facts.length >= limits.maxFacts) { + return reject("count_exceeded", true); + } // Retirement applies only to earlier accepted connector facts. Unknown ids are a no-op. const retired: string[] = []; for (const id of fact.resolves ?? []) { diff --git a/reference-implementation/server/routes/ref-run-observations.ts b/reference-implementation/server/routes/ref-run-observations.ts index b35cb2eeb..c2c3f0257 100644 --- a/reference-implementation/server/routes/ref-run-observations.ts +++ b/reference-implementation/server/routes/ref-run-observations.ts @@ -12,7 +12,10 @@ // the owner session that asked, and nothing else reads it. import type { ConnectorObservationRunStore } from "../stores/connector-observation-store.ts"; -import { getDefaultConnectorObservationStore } from "../stores/connector-observation-store.ts"; +import { + getDefaultConnectorObservationStore, + OBSERVATION_IDENTITY_MAX_LENGTH, +} from "../stores/connector-observation-store.ts"; import type { MiddlewareHandler, PdppErrorFn } from "./_route-contract.ts"; interface RouteRequest { @@ -39,6 +42,18 @@ export interface MountRefRunObservationsContext { store?: ConnectorObservationRunStore; } +/** Decodes the path segment. Malformed percent-encoding is null, not an exception. */ +function decodeRunId(raw: unknown): string | null { + if (typeof raw !== "string") { + return null; + } + try { + return decodeURIComponent(raw); + } catch { + return null; + } +} + function requiredQueryString(query: RouteRequest["query"], key: string): string | null { const value = query?.[key]; return typeof value === "string" && value.length > 0 ? value : null; @@ -48,7 +63,10 @@ export function mountRefRunObservations(app: AppLike, ctx: MountRefRunObservatio app.get("/_ref/runs/:runId/observations", ctx.requireOwnerSession, async (req: RouteRequest, res: RouteResponse) => { res.setHeader("Cache-Control", "no-store"); try { - const runId = decodeURIComponent(req.params.runId as string); + const runId = decodeRunId(req.params.runId); + if (runId === null) { + return ctx.pdppError(res, 400, "invalid_request", "run_id is not a valid identifier", "run_id"); + } const connectorId = requiredQueryString(req.query, "connector_id"); const connectorInstanceId = requiredQueryString(req.query, "connector_instance_id"); if (connectorId === null) { @@ -58,6 +76,17 @@ export function mountRefRunObservations(app: AppLike, ctx: MountRefRunObservatio return ctx.pdppError(res, 400, "invalid_request", "connector_instance_id is required", "connector_instance_id"); } + // The store would reject an over-long identity as an internal error. It is the caller's mistake. + for (const [param, value] of [ + ["connector_id", connectorId], + ["connector_instance_id", connectorInstanceId], + ["run_id", runId], + ] as const) { + if (value.length > OBSERVATION_IDENTITY_MAX_LENGTH || value.trim().length === 0) { + return ctx.pdppError(res, 400, "invalid_request", `${param} is not a valid identifier`, param); + } + } + const store = ctx.store ?? getDefaultConnectorObservationStore(); const record = await store.get({ connectorId, connectorInstanceId, runId }); if (record === null) { diff --git a/reference-implementation/server/stores/connector-observation-store.ts b/reference-implementation/server/stores/connector-observation-store.ts index 06df970ac..aa815c00c 100644 --- a/reference-implementation/server/stores/connector-observation-store.ts +++ b/reference-implementation/server/stores/connector-observation-store.ts @@ -29,7 +29,12 @@ import { execDynamicSqlAcknowledged, iterateDynamicSqlAcknowledged, writeTransaction } from "../../lib/db.ts"; import { RECOVERY_ACTIONS } from "../../runtime/connector-gap-bounding.ts"; -import type { Diagnosis, ObservationSnapshot } from "../../runtime/observations.ts"; +import { + type Diagnosis, + isObservedFact, + isRuntimeDefect, + type ObservationSnapshot, +} from "../../runtime/observations.ts"; import { getStorageBackendKind, isPostgresStorageBackend, @@ -41,7 +46,7 @@ export const CONNECTOR_OBSERVATION_RUNS_PER_CONNECTOR = 8; export const CONNECTOR_OBSERVATION_RECORD_MAX_BYTES = 1024 * 1024; const RECORD_VERSION = 1; -const MAX_IDENTITY_LENGTH = 256; +export const OBSERVATION_IDENTITY_MAX_LENGTH = 256; const DIAGNOSIS_CATEGORIES: ReadonlySet = new Set(["auth_rejected", "expectation_mismatch", "unknown"]); const DIAGNOSIS_ATTRIBUTIONS: ReadonlySet = new Set(["connector", "none"]); const LOCK_NAMESPACE = "connector_observation_runs:"; @@ -163,6 +168,11 @@ function isSnapshot(value: unknown): value is ObservationSnapshot { ); } +/** Element-level check. It runs after the size bound, so a huge record is refused before it is validated. */ +function hasValidElements(snapshot: ObservationSnapshot): boolean { + return snapshot.facts.every(isObservedFact) && snapshot.defects.every(isRuntimeDefect); +} + function isConnectorSuggestions(value: unknown): boolean { return ( Array.isArray(value) && @@ -196,9 +206,9 @@ function invalid(message: string): ConnectorObservationStoreError { } function requireIdentity(field: string, value: unknown): string { - if (typeof value !== "string" || value.trim().length === 0 || value.length > MAX_IDENTITY_LENGTH) { + if (typeof value !== "string" || value.trim().length === 0 || value.length > OBSERVATION_IDENTITY_MAX_LENGTH) { throw invalid( - `connector observation ${field} must be a non-blank string of at most ${MAX_IDENTITY_LENGTH} characters` + `connector observation ${field} must be a non-blank string of at most ${OBSERVATION_IDENTITY_MAX_LENGTH} characters` ); } return value; @@ -246,12 +256,22 @@ function encodeRecord(input: ConnectorObservationRunInput): { `connector observation record exceeds ${CONNECTOR_OBSERVATION_RECORD_MAX_BYTES} bytes` ); } + if (!hasValidElements(input.snapshot)) { + throw invalid("connector observation facts and defects must match the runtime shapes"); + } return { key, payload }; } function decodeRow(row: RunRow): ConnectorObservationRunRecord { - const parsed: unknown = JSON.parse(row.payload_json); - if (!(isRecord(parsed) && isSnapshot(parsed.snapshot) && isDiagnosis(parsed.diagnosis))) { + let parsed: unknown; + try { + parsed = JSON.parse(row.payload_json); + } catch { + throw invalid("stored connector observation record is malformed"); + } + if ( + !(isRecord(parsed) && isSnapshot(parsed.snapshot) && hasValidElements(parsed.snapshot) && isDiagnosis(parsed.diagnosis)) + ) { throw invalid("stored connector observation record is malformed"); } return { @@ -264,6 +284,22 @@ function decodeRow(row: RunRow): ConnectorObservationRunRecord { }; } +/** + * A list must not fail for one unreadable row. The row is skipped, and a direct + * `get` of its key still reports `record_invalid`. + */ +function decodeReadableRows(rows: readonly RunRow[]): ConnectorObservationRunRecord[] { + const records: ConnectorObservationRunRecord[] = []; + for (const row of rows) { + try { + records.push(decodeRow(row)); + } catch { + // Skipped on purpose; see above. + } + } + return records; +} + function assertSamePayload(stored: string, incoming: string): void { if (stored !== incoming) { throw new ConnectorObservationStoreError( @@ -314,7 +350,7 @@ function createSqliteConnectorObservationRunStore(): ConnectorObservationRunStor limit, ]), ]; - return rows.map(decodeRow); + return decodeReadableRows(rows); }); }, put(input: ConnectorObservationRunInput): Promise { @@ -381,7 +417,7 @@ function createPostgresConnectorObservationRunStore(): ConnectorObservationRunSt requireIdentity("connectorInstanceId", input.connectorInstanceId), limit, ]); - return result.rows.map(decodeRow); + return decodeReadableRows(result.rows); }, async put(input: ConnectorObservationRunInput): Promise { const { key, payload } = encodeRecord(input); diff --git a/reference-implementation/test/connector-observation-store.test.ts b/reference-implementation/test/connector-observation-store.test.ts index 59b9e0f4d..5e01d6b27 100644 --- a/reference-implementation/test/connector-observation-store.test.ts +++ b/reference-implementation/test/connector-observation-store.test.ts @@ -46,6 +46,7 @@ type FaultKind = "prune" | "insert"; /** One storage engine under test. Both implementations expose the same seams. */ interface Harness { + corruptPayload: (connectorId: string, runId: string, payloadJson: string) => Promise; deleteConnection: (connectorInstanceId: string) => Promise; installFault: (kind: FaultKind) => Promise; removeFault: (kind: FaultKind) => Promise; @@ -170,6 +171,12 @@ function faultSql(kind: FaultKind, action: "install" | "remove"): string[] { function sqliteHarness(path: string): Harness { return { + corruptPayload: (connectorId, runId, payloadJson) => + settle(() => { + getDb() + .prepare("UPDATE connector_observation_runs SET payload_json = ? WHERE connector_id = ? AND run_id = ?") + .run(payloadJson, connectorId, runId); + }), deleteConnection: (connectorInstanceId) => settle(() => { getDb().prepare("DELETE FROM connector_instances WHERE connector_instance_id = ?").run(connectorInstanceId); @@ -227,6 +234,12 @@ function sqliteHarness(path: string): Harness { function postgresHarness(databaseUrl: string): Harness { return { + async corruptPayload(connectorId, runId, payloadJson) { + await postgresQuery( + "UPDATE connector_observation_runs SET payload_json = $1 WHERE connector_id = $2 AND run_id = $3", + [payloadJson, connectorId, runId] + ); + }, async deleteConnection(connectorInstanceId) { await postgresQuery("DELETE FROM connector_instances WHERE connector_instance_id = $1", [connectorInstanceId]); }, @@ -756,6 +769,52 @@ async function contractConcurrentPutsKeepRetention(harness: Harness): Promise { + const c = uniq("c-elements"); + const i = uniq("i-elements"); + await harness.seedConnection(c, i); + const base = runInput(c, i, "run-elements"); + const goodFact = base.snapshot.facts[0]; + const goodDefect = base.snapshot.defects[0]; + assert.ok(goodFact && goodDefect, "the runtime fixture has a fact and a defect"); + const malformed: [string, ConnectorObservationRunInput["snapshot"]][] = [ + ["a string fact", { ...base.snapshot, facts: ["x"] } as never], + ["a null fact", { ...base.snapshot, facts: [null] } as never], + ["a fact of an unknown type", { ...base.snapshot, facts: [{ ...goodFact, fact: "credential_submission" }] } as never], + ["a fact with free text in its attrs", { ...base.snapshot, facts: [{ ...goodFact, attrs: { note: "private text" } }] } as never], + ["a fact without a retired flag", { ...base.snapshot, facts: [{ ...goodFact, retired: undefined }] } as never], + ["a number defect", { ...base.snapshot, defects: [7] } as never], + ["a defect of an unknown class", { ...base.snapshot, defects: [{ ...goodDefect, class: "free_text" }] } as never], + ["a defect carrying connector text", { ...base.snapshot, defects: [{ ...goodDefect, message: "private text" }] } as never], + ]; + await inOrder(malformed, ([label, snapshot]) => + expectStoreError(storeOf().put({ ...base, runId: "run-bad-element", snapshot }), "record_invalid", label) + ); + assert.equal(await harness.rowCount(c), 0, "a rejected record mutates nothing"); + await storeOf().put(base); + assert.equal(await harness.rowCount(c), 1, "the runtime-built record is still accepted"); +} + +async function contractListSkipsCorruptRows(harness: Harness): Promise { + const c = uniq("c-corrupt"); + const i = uniq("i-corrupt"); + await seedAndPut(harness, c, i, ["run-01", "run-02", "run-03"]); + await harness.corruptPayload(c, "run-02", '{"snapshot":{"facts":["x"],"defects":[],"truncated":false}}'); + const listed = await storeOf().list({ connectorId: c }); + assert.deepEqual( + listed.map((record) => record.runId), + ["run-03", "run-01"], + "one corrupt row must not hide the connector's other records" + ); + await expectStoreError(storeOf().get({ connectorId: c, connectorInstanceId: i, runId: "run-02" }), "record_invalid", "a direct read of the corrupt row fails"); + await harness.corruptPayload(c, "run-03", "not json"); + assert.deepEqual( + (await storeOf().list({ connectorId: c })).map((record) => record.runId), + ["run-01"], + "an unparseable payload is skipped the same way" + ); +} + test("SQLite connector observation store: persists, reopens and reads back the exact snapshot", async () => { await withSqlite(contractPersistsExactSnapshot); }); @@ -788,6 +847,26 @@ test("SQLite connector observation store: deleting a connection cascades its run await withSqlite(contractDeletingConnectionCascades); }); +test("SQLite connector observation store: rejects malformed fact and defect elements", async () => { + await withSqlite(contractRejectsMalformedElements); +}); + +test("SQLite connector observation store: list skips a corrupt stored row", async () => { + await withSqlite(contractListSkipsCorruptRows); +}); + +test("Postgres connector observation store: rejects malformed fact and defect elements", { + skip: POSTGRES_URL ? false : "PDPP_TEST_POSTGRES_URL unset", +}, async () => { + await withPostgres(contractRejectsMalformedElements); +}); + +test("Postgres connector observation store: list skips a corrupt stored row", { + skip: POSTGRES_URL ? false : "PDPP_TEST_POSTGRES_URL unset", +}, async () => { + await withPostgres(contractListSkipsCorruptRows); +}); + test("Postgres connector observation store: persists, reopens and reads back the exact snapshot", { skip: POSTGRES_URL ? false : "PDPP_TEST_POSTGRES_URL unset", }, async () => { diff --git a/reference-implementation/test/ref-run-observations-route.test.ts b/reference-implementation/test/ref-run-observations-route.test.ts index dc7b05b6c..ca5207a5c 100644 --- a/reference-implementation/test/ref-run-observations-route.test.ts +++ b/reference-implementation/test/ref-run-observations-route.test.ts @@ -188,3 +188,19 @@ test("the route requires both namespace query parameters", async () => { const missingInstance = await invoke("owner", { connector_id: CONNECTOR_ID }, fakeStore(fakeRecord())); assert.equal(missingInstance.status, 400); }); + +test("a malformed percent-encoded run id is a 400, not a server error", async () => { + const result = await invoke("owner", OWNER_QUERY, fakeStore(fakeRecord()), "%E0%A4%A"); + assert.equal(result.status, 400); + assert.equal((result.body?.error as { code?: string } | undefined)?.code, "invalid_request"); +}); + +test("an over-long connector or instance id is a 400, not a server error", async () => { + const tooLong = "x".repeat(257); + const connector = await invoke("owner", { ...OWNER_QUERY, connector_id: tooLong }, fakeStore(fakeRecord())); + assert.equal(connector.status, 400); + const instance = await invoke("owner", { ...OWNER_QUERY, connector_instance_id: tooLong }, fakeStore(fakeRecord())); + assert.equal(instance.status, 400); + const run = await invoke("owner", OWNER_QUERY, fakeStore(fakeRecord()), tooLong); + assert.equal(run.status, 400); +}); diff --git a/reference-implementation/test/runtime-observations.test.ts b/reference-implementation/test/runtime-observations.test.ts index ec0c9fd28..45e411098 100644 --- a/reference-implementation/test/runtime-observations.test.ts +++ b/reference-implementation/test/runtime-observations.test.ts @@ -227,25 +227,83 @@ test("hasEmittedId reports rejected ids as well as accepted ones", () => { assert.equal(run.hasEmittedId("never"), false); }); -test("a failed DONE citing an id past the full id ledger is kept without the unverifiable id", () => { +function doneContract(run: ObservationRun) { + return { + hasEmittedId: (id: string) => run.hasEmittedId(id), + idLedgerOverflowed: run.idLedgerOverflowed(), + observationDeclared: true, + strict: true, + }; +} + +test("a failed DONE citing an id past the full discard ledger is kept without the unverifiable id", () => { + const { run } = harness({ limits: { maxFacts: 64, maxPerWindow: 1000 } }); + run.accept(authFailure("c0")); + // Invalid messages still emit their ids (Sections 5.8 and 5.10.2). The ledger + // for them is bounded, so the 65th id is emitted but cannot be kept. + for (let i = 0; i < 65; i += 1) { + assert.equal(rejectedReason(run.accept({ fact: "nope", id: `d${i}`, type: "OBSERVATION" })), "invalid"); + } + assert.equal(run.idLedgerOverflowed(), true); + assert.equal(run.hasEmittedId("d64"), false); + const result = validateDoneError("failed", { basis: ["c0", "d64"], message: "stopped", retryable: false }, doneContract(run)); + assert.ok(!(result instanceof Error), "a connector that overflowed the ledger must still get its terminal outcome"); + assert.deepEqual(result?.basis, ["c0"]); + // The dropped id names no stored fact, so dropping it cannot change the diagnosis. + assert.deepEqual(run.diagnose(result?.basis ?? []), run.diagnose(["c0", "d64"])); + assert.equal(run.diagnose(result?.basis ?? []).category, "auth_rejected"); +}); + +test("a valid fact refused for the fact limit still counts as emitted and its id stays citable", () => { const { run } = harness({ limits: { maxFacts: 64, maxPerWindow: 1000 } }); for (let i = 0; i < 64; i += 1) { assert.equal(run.accept(waitExpired(`c${i}`)).status, "accepted"); } - // The connector did emit this id, but the bounded ledger could not keep it. assert.equal(rejectedReason(run.accept(waitExpired("c64"))), "count_exceeded"); - const result = validateDoneError( - "failed", - { basis: ["c0", "c64"], message: "stopped", retryable: false }, - { - hasEmittedId: (id) => run.hasEmittedId(id), - idLedgerOverflowed: run.idLedgerOverflowed(), - observationDeclared: true, - strict: true, - } - ); - assert.ok(!(result instanceof Error), "a connector that overflowed the ledger must still get its terminal outcome"); - assert.deepEqual(result?.basis, ["c0"]); + assert.equal(run.idLedgerOverflowed(), false, "refusing a valid fact is not a ledger overflow"); + const result = validateDoneError("failed", { basis: ["c0", "c64"], message: "stopped", retryable: false }, doneContract(run)); + assert.ok(!(result instanceof Error)); + assert.deepEqual(result?.basis, ["c0", "c64"]); +}); + +test("invalid traffic does not spend the budget that accepted facts need", () => { + const { run } = harness({ limits: { maxFacts: 64, maxPerWindow: 64 } }); + for (let i = 0; i < 500; i += 1) { + assert.equal(rejectedReason(run.accept({ fact: "nope", id: `bad${i}`, type: "OBSERVATION" })), "invalid"); + } + for (let i = 0; i < 64; i += 1) { + assert.equal(run.accept(waitExpired(`ok${i}`)).status, "accepted", `valid fact ${i} must not be evicted by invalid traffic`); + } + assert.equal(run.snapshot().facts.length, 64); + assert.equal(run.snapshot().truncated, false, "no valid fact was lost, so the record is not truncated"); + assert.deepEqual(run.gateRecoveryHint("not_retriable", "skip_result"), { result: "authorized" }); +}); + +test("a line that is not JSON never marks the record truncated, however long it is", () => { + const { run } = harness(); + assert.equal(rejectedReason(run.acceptLine("x".repeat(5000))), "oversize"); + assert.equal(run.snapshot().truncated, false); + assert.deepEqual(run.gateRecoveryHint("not_retriable", "skip_result"), { result: "authorized" }); +}); + +test("a duplicate id is reported as duplicate_id even when the repeat is also oversize", () => { + const { run } = harness(); + assert.equal(run.accept(waitExpired("dup")).status, "accepted"); + assert.equal(rejectedReason(run.accept({ ...waitExpired("dup"), padding: "x".repeat(3000) })), "duplicate_id"); + // An oversize first sighting still uses its id. + assert.equal(rejectedReason(run.accept({ ...waitExpired("big"), padding: "x".repeat(3000) })), "oversize"); + assert.equal(rejectedReason(run.accept(waitExpired("big"))), "duplicate_id"); +}); + +test("a DONE basis with a malformed id names the syntax problem, not a missing emission", () => { + const { run } = harness(); + run.accept(waitExpired("c0")); + const malformed = validateDoneError("failed", { basis: ["Not A Valid Id"], message: "stopped", retryable: false }, doneContract(run)); + const missing = validateDoneError("failed", { basis: ["never"], message: "stopped", retryable: false }, doneContract(run)); + assert.ok(malformed instanceof Error && missing instanceof Error); + assert.notEqual(malformed.message, missing.message); + assert.match(malformed.message, /not a valid OBSERVATION id/); + assert.equal(malformed.message.includes("Not A Valid Id"), false, "the rejected id is not echoed"); }); test("a failed DONE citing an id the connector never emitted is still rejected while the ledger has room", () => { From 7a284ec7d1d9f076c157a09a246b7ba19e63463f Mon Sep 17 00:00:00 2001 From: Tim Nunamaker Date: Fri, 6 Nov 2026 00:03:48 -0600 Subject: [PATCH 6/9] fix(runtime): withhold unparseable connector lines and seal cancellation at the record write An independent audit of the OBSERVATION intake found four defects in this PR's own code. Unparseable lines (runtime/index.ts): - A line that does not parse was classified by a text match on `"type":"OBSERVATION"`. A JSON-escaped spelling of the type missed the match, so the whole line went to the trace and into the parse-error message. Nothing is known about the content of a line that does not parse, so it is never traced (the trace gets its byte count) and neither its text nor the parser message, which can quote the source, goes into an error. The error carries the line length only. - The line is parsed once. The result goes to a single intake, `acceptWire`, which `acceptLine` also uses. Before, the runtime called `accept(undefined, bytes)` for a line that did not parse, which marked the record truncated, while `acceptLine` did not. Cancellation (runtime/index.ts): - The diagnosis was computed from the cancel flag before the record write, and the terminal outcome read the flag again after it. A cancel that landed during the write gave a cancelled run with a stored `auth_rejected` cause. Starting the write now seals the owner-cancel state. A later owner cancel stops the child but does not change the sealed outcome. Store (server/stores/connector-observation-store.ts): - The PostgreSQL write waited on the connector's advisory lock with no bound, and the runtime waits for the write before it publishes a terminal result. The transaction now sets a transaction-local `lock_timeout` and `statement_timeout` (5 s), so a stalled writer fails one optional put and the server cancels the query. Tests: `collection-profile-020` sends raw wire lines (the escaped-type line and a 3 KB non-JSON line) and aborts during the store write; `connector-observation-store` holds the writer lock from another transaction (PostgreSQL); `runtime-observations` checks `acceptWire` against `acceptLine`. Each new test fails before its fix. Assisted-by: AI Signed-off-by: Tim Nunamaker --- reference-implementation/runtime/index.ts | 99 ++++++++++--------- .../runtime/observations.ts | 29 ++++-- .../stores/connector-observation-store.ts | 23 ++++- .../test/collection-profile-020.test.ts | 94 +++++++++++++++++- .../test/connector-observation-store.test.ts | 49 ++++++++- .../test/runtime-observations.test.ts | 11 +++ 6 files changed, 245 insertions(+), 60 deletions(-) diff --git a/reference-implementation/runtime/index.ts b/reference-implementation/runtime/index.ts index 6fb1c32ac..fbb7fcd86 100644 --- a/reference-implementation/runtime/index.ts +++ b/reference-implementation/runtime/index.ts @@ -106,7 +106,7 @@ import { } from "./done-validators.ts"; import type { IngestResult as StrictIngestResult } from "./ingest-failure.ts"; import { readIngestResponse as strictReadIngestResponse } from "./ingest-failure.ts"; -import { createObservationRun } from "./observations.ts"; +import { createObservationRun, type WireParse } from "./observations.ts"; import { buildHttpFailure, buildIngestEnvelopeContractViolationFailure, @@ -3089,7 +3089,7 @@ export async function runConnector(opts: RuntimeRunConnectorOptions): Promise(); + const observationInputs = new WeakMap(); const explicitlyRequestedStreams = requestedRuntimeStreams(providedScope); const declaredCollectionScope = readStoredCollectionScope(state).scope; @@ -3730,6 +3730,9 @@ export async function runConnector(opts: RuntimeRunConnectorOptions): Promise | null = null; let runTimedOut = false; let ownerCancelForced = false; const terminalStopRequested = (): boolean => ownerCancelRequested || runTimedOut; @@ -5131,6 +5134,14 @@ export async function runConnector(opts: RuntimeRunConnectorOptions): Promise | null = null; - /** * The owner diagnostic record (Section 5.10 and 5.11), written once per run. * The first settle point starts it, and every settle point awaits it, so the @@ -6557,6 +6566,10 @@ export async function runConnector(opts: RuntimeRunConnectorOptions): Promise { + // Starting the write seals the owner-cancel state: the diagnosis below and + // the terminal outcome both read it, and `handleCancellation` stops changing + // it from here on. + ownerCancelSealed = true; observationDiagnosisWrite ??= getDefaultConnectorObservationStore() .put({ connectorId, @@ -6606,71 +6619,61 @@ export async function runConnector(opts: RuntimeRunConnectorOptions): Promise { + if (!line.trim()) { + writeTrace(line); + return; + } // Parse once. A line is an OBSERVATION by its parsed type, or by its text - // when it does not parse. Raw OBSERVATION lines are never traced, logged, - // spined or webhooked (Section 5.10). - let parsed: unknown; - let parseError: RuntimeRunError | null = null; + // when it does not parse. Nothing is known about the content of a line that + // does not parse, so it is never traced and its text or parser message is + // never put in an error: only its length is (Section 5.10). + let wire: WireParse; try { - parsed = JSON.parse(line); - } catch (caught) { - parseError = caught as RuntimeRunError; + wire = { ok: true, value: JSON.parse(line) }; + } catch { + wire = { ok: false }; } - const observation = parseError === null ? isObservationMessage(parsed) : OBSERVATION_TYPE_TEXT_RE.test(line); - if (!observation) { + const observation = wire.ok ? isObservationMessage(wire.value) : OBSERVATION_TYPE_TEXT_RE.test(line); + if (!wire.ok) { + writeTrace(`[unparseable line: ${Buffer.byteLength(line, "utf8")} bytes withheld]`); + } else if (!observation) { writeTrace(line); } - if (!line.trim()) { - return; - } if (observation) { if (failPendingInteraction(new Error("Connector emitted OBSERVATION while waiting for INTERACTION_RESPONSE"))) { return; } const envelope: ConnectorMessage = { type: OBSERVATION_MESSAGE_TYPE }; - observationInputs.set(envelope, { value: parsed, serializedBytes: Buffer.byteLength(line, "utf8") }); + observationInputs.set(envelope, { parse: wire, serializedBytes: Buffer.byteLength(line, "utf8") }); msgQueue.push(envelope); processNext().catch(rejectAfterDiagnosis); return; } - try { - if (parseError !== null) { - throw parseError; - } - const msg = parsed as ConnectorMessage; - if (resolvePendingOtpInteraction(msg)) { - // The matching resolution releases the protocol gate immediately. - // DONE can arrive on the next line before the async interaction - // completion clears pendingInteraction. - return; - } - if (failPendingInteraction(new Error(`Connector emitted ${msg.type} while waiting for INTERACTION_RESPONSE`))) { - return; - } - msgQueue.push(msg); - processNext().catch(rejectAfterDiagnosis); - } catch (caught) { - const err = caught as RuntimeRunError; + if (!wire.ok || typeof wire.value !== "object" || wire.value === null) { + const detail = `${wire.ok ? "not a JSON object" : "not valid JSON"} (line_length=${line.length})`; if ( failPendingInteraction( - new Error(`Connector emitted invalid JSONL while waiting for INTERACTION_RESPONSE: ${err.message}`) + new Error(`Connector emitted invalid JSONL while waiting for INTERACTION_RESPONSE: ${detail}`) ) ) { return; } - // Context for debugging: include byte length and a preview of the - // offending line in the error message. - const preview = - line.length > 400 - ? `${line.slice(0, 200)} … [truncated ${line.length - 400} chars] … ${line.slice(-200)}` - : line; - const enriched = `${err.message} (line_length=${line.length} preview=${JSON.stringify(preview).slice(0, 600)})`; - msgQueue.push({ - error: enriched, - type: "__PARSE_ERROR__", - }); + msgQueue.push({ error: detail, type: "__PARSE_ERROR__" }); processNext().catch(rejectAfterDiagnosis); + return; + } + const msg = wire.value as ConnectorMessage; + if (resolvePendingOtpInteraction(msg)) { + // The matching resolution releases the protocol gate immediately. + // DONE can arrive on the next line before the async interaction + // completion clears pendingInteraction. + return; + } + if (failPendingInteraction(new Error(`Connector emitted ${msg.type} while waiting for INTERACTION_RESPONSE`))) { + return; } + msgQueue.push(msg); + processNext().catch(rejectAfterDiagnosis); }); async function recordRunTimedOutTerminal(code: number | null): Promise { diff --git a/reference-implementation/runtime/observations.ts b/reference-implementation/runtime/observations.ts index a56f21b50..19712ae5a 100644 --- a/reference-implementation/runtime/observations.ts +++ b/reference-implementation/runtime/observations.ts @@ -181,6 +181,9 @@ export interface CreateObservationRunInput { readonly now?: () => number; } +/** The result of parsing one wire line. A line that is not JSON carries no value. */ +export type WireParse = { readonly ok: true; readonly value: unknown } | { readonly ok: false }; + export interface ObservationRun { /** Runtime-owned raw byte count, or canonical JSON size for in-memory callers. */ accept: (value: unknown, serializedBytes?: number) => AcceptOutcome; @@ -190,6 +193,11 @@ export interface ObservationRun { * is read as a size. */ acceptLine: (line: string) => AcceptOutcome; + /** + * The same intake for a caller that has already parsed the line. `acceptLine` + * is this entry plus a parse, so both give one outcome for one wire line. + */ + acceptWire: (parse: WireParse, serializedBytes: number) => AcceptOutcome; diagnose: (basis?: readonly string[]) => Diagnosis; /** * Section 5.11 gate for one connector hint. `authorized` means the runtime @@ -528,19 +536,25 @@ export function createObservationRun(input: CreateObservationRunInput): Observat } function acceptLine(line: string): AcceptOutcome { + let parse: WireParse; + try { + parse = { ok: true, value: JSON.parse(line) }; + } catch { + parse = { ok: false }; + } + return acceptWire(parse, Buffer.byteLength(line, "utf8")); + } + + function acceptWire(parse: WireParse, serializedBytes: number): AcceptOutcome { if (!input.declared) { return reject("undeclared_capability", false); } - const bytes = Buffer.byteLength(line, "utf8"); - let value: unknown; - try { - value = JSON.parse(line); - } catch { + if (!parse.ok) { // Text that is not JSON is no fact, so no fact was lost: the oversize // reason is recorded, but the record is not marked truncated. - return bytes > limits.maxFactBytes ? reject("oversize", false) : reject("invalid", false); + return serializedBytes > limits.maxFactBytes ? reject("oversize", false) : reject("invalid", false); } - return accept(value, bytes); + return accept(parse.value, serializedBytes); } function accept(value: unknown, serializedBytes?: number): AcceptOutcome { @@ -700,6 +714,7 @@ export function createObservationRun(input: CreateObservationRunInput): Observat return { accept, acceptLine, + acceptWire, diagnose, gateRecoveryHint, hasEmittedId, diff --git a/reference-implementation/server/stores/connector-observation-store.ts b/reference-implementation/server/stores/connector-observation-store.ts index aa815c00c..8737fef3d 100644 --- a/reference-implementation/server/stores/connector-observation-store.ts +++ b/reference-implementation/server/stores/connector-observation-store.ts @@ -394,7 +394,7 @@ function createSqliteConnectorObservationRunStore(): ConnectorObservationRunStor }; } -function createPostgresConnectorObservationRunStore(): ConnectorObservationRunStore { +function createPostgresConnectorObservationRunStore(writeTimeoutMs: number): ConnectorObservationRunStore { return { async get(input: ConnectorObservationRunKey): Promise { const key = requireKey(input); @@ -423,6 +423,14 @@ function createPostgresConnectorObservationRunStore(): ConnectorObservationRunSt const { key, payload } = encodeRecord(input); const recordedAt = new Date().toISOString(); await withPostgresTransaction(async (client) => { + // The record is optional reporting, and the runtime waits for this write + // before it publishes a terminal result. Bound the wait for the writer lock + // and the work itself, so a stalled writer fails this put, and the server + // cancels its own query. Transaction-local, so the pooled client is not + // left with the setting. + await client.query("SELECT set_config('lock_timeout', $1, true), set_config('statement_timeout', $1, true)", [ + String(writeTimeoutMs), + ]); // Transaction-scoped, so it releases at COMMIT or ROLLBACK, and it // serializes writers in every process that shares this database. await client.query("SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", [ @@ -458,9 +466,18 @@ function createPostgresConnectorObservationRunStore(): ConnectorObservationRunSt }; } -export function createConnectorObservationRunStore(): ConnectorObservationRunStore { +/** How long one PostgreSQL write may wait for the writer lock or run, in milliseconds. */ +export const CONNECTOR_OBSERVATION_WRITE_TIMEOUT_MS = 5000; + +export interface ConnectorObservationRunStoreOptions { + readonly writeTimeoutMs?: number; +} + +export function createConnectorObservationRunStore( + options: ConnectorObservationRunStoreOptions = {} +): ConnectorObservationRunStore { return isPostgresStorageBackend() - ? createPostgresConnectorObservationRunStore() + ? createPostgresConnectorObservationRunStore(options.writeTimeoutMs ?? CONNECTOR_OBSERVATION_WRITE_TIMEOUT_MS) : createSqliteConnectorObservationRunStore(); } diff --git a/reference-implementation/test/collection-profile-020.test.ts b/reference-implementation/test/collection-profile-020.test.ts index df09a09e5..26f52b837 100644 --- a/reference-implementation/test/collection-profile-020.test.ts +++ b/reference-implementation/test/collection-profile-020.test.ts @@ -80,6 +80,7 @@ interface Stub { readonly markerPath: string; } +// A string message is written to stdout as is, so a test can send a line that is not valid JSON. // The child writes a marker file as soon as it receives START. A test that // expects no spawn checks that the marker is absent. function makeStub(messages: readonly unknown[], exitCode: number): Stub { @@ -95,7 +96,7 @@ const rl = createInterface({ input: process.stdin, terminal: false }); rl.on('line', (line) => { if (JSON.parse(line).type !== 'START') return; writeFileSync(${JSON.stringify(markerPath)}, line); - for (const message of ${JSON.stringify(messages)}) process.stdout.write(JSON.stringify(message) + '\\n'); + for (const message of ${JSON.stringify(messages)}) process.stdout.write((typeof message === 'string' ? message : JSON.stringify(message)) + '\\n'); rl.close(); process.exit(${exitCode}); }); @@ -395,6 +396,54 @@ test("raw OBSERVATION lines, including rejected ones, never reach the runtime tr } }); +test("a malformed line never reaches the trace or the run error, whatever its type spelling", async () => { + // The type is JSON-escaped and the closing brace is missing: the line does not + // parse, and a text match on "type":"OBSERVATION" cannot classify it. It must be + // treated as possibly private, not as trace-safe. + const traceDir = mkdtempSync(join(tmpdir(), "pdpp-collection-profile-020-trace-")); + const stub = makeStub( + [ + `{"type":"\\u004fBSERVATION","private":"${SECRET_MARKER}"`, + { records_emitted: 0, status: "succeeded", type: "DONE" }, + ], + 0 + ); + try { + const outcome = await runStub(stub, "run_c020_escaped_malformed", manifest(), { PDPP_TRACE_DIR: traceDir }); + const traceText = readdirSync(traceDir) + .map((name) => readFileSync(join(traceDir, name), "utf8")) + .join("\n"); + assert.ok(traceText.includes("pdpp-runtime-trace"), "the trace file must exist, so the absence below is meaningful"); + assert.equal(traceText.includes(SECRET_MARKER), false, "an unparseable line must not be traced verbatim"); + assert.equal(JSON.stringify(outcome).includes(SECRET_MARKER), false, "the run error must not carry the line"); + assert.equal(outcome.ok, false, "an unparseable line that names no OBSERVATION is still a protocol violation"); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + rmSync(traceDir, { force: true, recursive: true }); + } +}); + +test("an oversize line that is not JSON records oversize without marking the record truncated", async () => { + // This goes through the runtime's child stdout path, not the unit entry. + const stub = makeStub( + [ + `{"type":"OBSERVATION","id":"junk1","pad":"${"x".repeat(3000)}`, + { records_emitted: 0, status: "succeeded", type: "DONE" }, + ], + 0 + ); + try { + const outcome = await runStub(stub, "run_c020_oversize_nonjson", manifest()); + assert.equal(outcome.ok, true, `run must succeed: ${outcome.ok ? "" : outcome.message}`); + const record = await persistedRecord("run_c020_oversize_nonjson"); + assert.ok(record, "the diagnostic record must be persisted"); + assert.deepEqual(record.snapshot.defects.map(defectKey), ["oversize"]); + assert.equal(record.snapshot.truncated, false, "text that is not JSON is no fact, so no fact was lost"); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } +}); + test("a declared-profile run whose manifest names an unknown profile version never spawns the child", async () => { const stub = makeStub([{ records_emitted: 0, status: "succeeded", type: "DONE" }], 0); try { @@ -578,6 +627,49 @@ test("a cancelled run stores no cause, even when its failed DONE cited an auth f } }); +test("a cancel that lands while the record is being stored cannot split the record from the terminal outcome", async () => { + // The owner record is written once, before the terminal event. The terminal + // outcome and the record's diagnosis are decided together at that write, so an + // abort that arrives during the write changes neither: the run ends as its + // failed DONE said, and its record keeps the cause that DONE cited. + const stub = makeStub( + [ + PROVIDER_REJECTED("y1"), + { + error: { basis: ["y1"], code: "credential_rejected", message: "refused", retryable: false }, + records_emitted: 0, + status: "failed", + type: "DONE", + }, + ], + 1 + ); + const abort = new AbortController(); + const store = getDefaultConnectorObservationStore(); + const put = store.put.bind(store); + const spy = mock.method(store, "put", async (input: Parameters[0]) => { + abort.abort(); + return put(input); + }); + try { + await runStub(stub, "run_c020_cancel_during_put", manifest(), {}, { cancelSignal: abort.signal }); + } finally { + spy.mock.restore(); + } + try { + const record = await persistedRecord("run_c020_cancel_during_put"); + assert.ok(record, "the diagnostic record must be persisted"); + const page = await listSpineEventsPage("run", "run_c020_cancel_during_put", { limit: 500 }); + const terminal = page.events.map((event) => event.event_type); + assert.equal(terminal.includes("run.cancelled"), false, "a cancel after the record is sealed does not cancel the run"); + assert.equal(terminal.includes("run.failed"), true); + assert.equal(record.diagnosis.category, "auth_rejected"); + assert.deepEqual(record.diagnosis.basis, ["y1"]); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } +}); + test("uncited, retired and undeclared-rule facts cannot support a cause", async () => { const stub = makeStub( [ diff --git a/reference-implementation/test/connector-observation-store.test.ts b/reference-implementation/test/connector-observation-store.test.ts index 5e01d6b27..19136ae58 100644 --- a/reference-implementation/test/connector-observation-store.test.ts +++ b/reference-implementation/test/connector-observation-store.test.ts @@ -24,13 +24,19 @@ import { validateDiagnosticDescriptors, } from "../runtime/observations.ts"; import { closeDb, getDb, initDb } from "../server/db.ts"; -import { closePostgresStorage, initPostgresStorage, postgresQuery } from "../server/postgres-storage.ts"; +import { + closePostgresStorage, + initPostgresStorage, + postgresQuery, + withPostgresTransaction, +} from "../server/postgres-storage.ts"; import { CONNECTOR_OBSERVATION_RECORD_MAX_BYTES, CONNECTOR_OBSERVATION_RUNS_PER_CONNECTOR, type ConnectorObservationRunInput, type ConnectorObservationRunStore, ConnectorObservationStoreError, + createConnectorObservationRunStore, getDefaultConnectorObservationStore, resetDefaultConnectorObservationStoreCache, } from "../server/stores/connector-observation-store.ts"; @@ -920,3 +926,44 @@ test("Postgres connector observation store: deleting a connection cascades its r }, async () => { await withPostgres(contractDeletingConnectionCascades); }); + +async function contractWriteWaitIsBounded(harness: Harness): Promise { + const c = uniq("c-lock"); + const i = uniq("i-lock"); + await harness.seedConnection(c, i); + const bounded = createConnectorObservationRunStore({ writeTimeoutMs: 300 }); + let release: () => void = () => undefined; + const held = new Promise((resolve) => { + release = resolve; + }); + let locked: () => void = () => undefined; + const lockTaken = new Promise((resolve) => { + locked = resolve; + }); + // Another transaction keeps this connector's writer lock, as a stalled writer would. + const holder = withPostgresTransaction(async (client) => { + await client.query("SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", [ + `connector_observation_runs:${c}`, + ]); + locked(); + await held; + }); + try { + await lockTaken; + const started = Date.now(); + await assert.rejects(bounded.put(runInput(c, i, "run-blocked")), "a put behind a stalled writer must fail"); + assert.ok(Date.now() - started < 5000, "the wait for the writer lock is bounded"); + } finally { + release(); + await holder; + } + await bounded.put(runInput(c, i, "run-after")); + assert.equal(await harness.rowCount(c), 1, "a later put succeeds once the lock is free"); +} + +test("Postgres connector observation store: a put behind a stalled writer fails within a bound", { + skip: POSTGRES_URL ? false : "PDPP_TEST_POSTGRES_URL unset", + timeout: 20_000, +}, async () => { + await withPostgres(contractWriteWaitIsBounded); +}); diff --git a/reference-implementation/test/runtime-observations.test.ts b/reference-implementation/test/runtime-observations.test.ts index 45e411098..6f5d0dd25 100644 --- a/reference-implementation/test/runtime-observations.test.ts +++ b/reference-implementation/test/runtime-observations.test.ts @@ -286,6 +286,17 @@ test("a line that is not JSON never marks the record truncated, however long it assert.deepEqual(run.gateRecoveryHint("not_retriable", "skip_result"), { result: "authorized" }); }); +test("acceptWire gives the same outcome as acceptLine for a line that is not JSON", () => { + // The runtime parses the wire line once and enters here, so both entries must agree. + const text = "x".repeat(5000); + const { run: viaLine } = harness(); + const { run: viaWire } = harness(); + assert.equal(rejectedReason(viaLine.acceptLine(text)), "oversize"); + assert.equal(rejectedReason(viaWire.acceptWire({ ok: false }, Buffer.byteLength(text, "utf8"))), "oversize"); + assert.equal(viaWire.snapshot().truncated, viaLine.snapshot().truncated); + assert.equal(viaWire.snapshot().truncated, false); +}); + test("a duplicate id is reported as duplicate_id even when the repeat is also oversize", () => { const { run } = harness(); assert.equal(run.accept(waitExpired("dup")).status, "accepted"); From 5425a4e84494a4ccafb3e2a192220a13075bdf56 Mon Sep 17 00:00:00 2001 From: Tim Nunamaker Date: Fri, 6 Nov 2026 05:31:35 -0600 Subject: [PATCH 7/9] refactor(runtime): move the recovery-hint gate out of the OBSERVATION PR The Section 5.11 gate (gateRecoveryHint, the suggestion record and the hint_unsupported defect) had no caller in this PR. It is code that nothing exercises, so it belongs with the change that wires it to DONE and SKIP_RESULT hints. That change is the next PR in the stack, which now carries the gate, its tests and its caller together. Removed here: gateRecoveryHint and HintGateOutcome, the action and suggestion types, the hint_unsupported defect subtype, the connector_suggestions snapshot field and its persistence validation in the observation store, and the tests for each. The store keeps writing only facts, defects and the truncation flag. The action vocabulary stays owned by RECOVERY_ACTIONS in connector-gap-bounding.ts; the second copy in observations.ts is gone with the gate. No behavior changes for any run: nothing called the gate. Signed-off-by: Tim Nunamaker Assisted-by: AI --- .../runtime/observations.ts | 117 +----------------- .../stores/connector-observation-store.ts | 24 +--- .../test/connector-observation-store.test.ts | 40 +----- .../test/runtime-observations.test.ts | 91 -------------- 4 files changed, 5 insertions(+), 267 deletions(-) diff --git a/reference-implementation/runtime/observations.ts b/reference-implementation/runtime/observations.ts index 19712ae5a..4de74b886 100644 --- a/reference-implementation/runtime/observations.ts +++ b/reference-implementation/runtime/observations.ts @@ -6,7 +6,7 @@ // // Normative sources: data-connectors docs spec sections 3.8 (diagnostic // descriptors), 5.10 (OBSERVATION facts, provenance, retirement, limits) and -// 5.11 (diagnosis and the recovery-hint gate). The wire grammar is the +// 5.11 (diagnosis). The wire grammar is the // canonical JSON Schema `observation.schema.json` beside this file, a byte // copy of data-connectors blob 8d9384d1f0827647e62e1914e8cac81d809034b3. The // schema cannot express the checks below, so this module adds them: @@ -108,41 +108,9 @@ export type RuntimeDefect = readonly class: "protocol_violation"; readonly reason: ObservationRejectReason; readonly count: number; - } - | { - readonly id: string; - readonly class: "hint_unsupported"; - readonly count: number; }; -/** The portable recovery actions of Collection Profile Section 5.5. */ -export type PortableRecoveryAction = - | "retry_by_runtime" - | "retry_on_connector_upgrade" - | "refresh_credentials" - | "manual_action_required" - | "update_selector" - | "upstream_unblock" - | "not_retriable" - | "unknown"; - -/** Where a connector hint came from. */ -export type RecoveryHintOrigin = "skip_result" | "done_error"; - -/** - * A connector hint the Section 5.11 gate did not authorize. It is shown only - * as the connector's attributed suggestion. The record holds closed values - * only: no free text, code, or retryable flag from the connector. - */ -export interface ConnectorSuggestion { - readonly action: PortableRecoveryAction; - readonly attribution: "connector"; - readonly origin: RecoveryHintOrigin; -} - export interface ObservationSnapshot { - /** Present only when at least one hint was not authorized. */ - readonly connector_suggestions?: readonly ConnectorSuggestion[]; readonly defects: readonly RuntimeDefect[]; readonly facts: readonly ObservedFact[]; readonly truncated: boolean; @@ -159,11 +127,6 @@ export interface Diagnosis { readonly contributing: readonly string[]; } -export type HintGateOutcome = - | { readonly result: "authorized" } - | { readonly result: "blocked"; readonly reason: "truncated" } - | { readonly result: "unsupported" }; - export interface ObservationLimits { /** Serialized bytes per OBSERVATION. Must be at least 2048. */ readonly maxFactBytes?: number; @@ -199,12 +162,6 @@ export interface ObservationRun { */ acceptWire: (parse: WireParse, serializedBytes: number) => AcceptOutcome; diagnose: (basis?: readonly string[]) => Diagnosis; - /** - * Section 5.11 gate for one connector hint. `authorized` means the runtime - * may act on the action. Any other result means the hint is shown only as - * the connector's suggestion, and the caller must not act on it. - */ - gateRecoveryHint: (action: string, origin: RecoveryHintOrigin) => HintGateOutcome; /** * True when the connector emitted this id earlier in the run, whether the * fact was accepted or rejected. The ledger is bounded by the fact limit. @@ -254,9 +211,6 @@ export function isRuntimeDefect(value: unknown): value is RuntimeDefect { if (!(Number.isSafeInteger(defect.count) && (defect.count as number) >= 1)) { return false; } - if (defect.class === "hint_unsupported") { - return hasOnlyKeys(defect, ["id", "class", "count"]) && defect.id === HINT_UNSUPPORTED_ID; - } return ( defect.class === "protocol_violation" && hasOnlyKeys(defect, ["id", "class", "reason", "count"]) && @@ -306,23 +260,6 @@ const RUNTIME_FACT_TYPES: ReadonlySet = new Set(["credential_submission const CATEGORY_ORDER = ["auth_rejected", "expectation_mismatch"] as const; -/** The closed Section 5.5 vocabulary. Suggestions are bounded by it: 8 actions times 2 origins. */ -const PORTABLE_RECOVERY_ACTIONS: ReadonlySet = new Set([ - "retry_by_runtime", - "retry_on_connector_upgrade", - "refresh_credentials", - "manual_action_required", - "update_selector", - "upstream_unblock", - "not_retriable", - "unknown", -]); - -// Hints the Section 5.11 gate holds in this version. Every other hint, portable -// or not, is unsupported. -const HINTS_THAT_HOLD: ReadonlySet = new Set(["not_retriable", "unknown"]); - -const HINT_UNSUPPORTED_ID = "runtime:connector_defect:hint_unsupported"; const PROTOCOL_VIOLATION_ID_PREFIX = "runtime:connector_defect:protocol_violation:"; // --------------------------------------------------------------------------- @@ -490,9 +427,6 @@ export function createObservationRun(input: CreateObservationRunInput): Observat const discardedIds = new Set(); let idLedgerOverflowed = false; const defects = new Map(); - // Keyed by origin and action. The key space is closed (2 origins x 8 - // actions), so the record is bounded without a cap. - const suggestions = new Map(); let truncated = false; let windowStart: number | undefined; let windowCount = 0; @@ -504,16 +438,6 @@ export function createObservationRun(input: CreateObservationRunInput): Observat defects.set(id, { class: "protocol_violation", count, id, reason }); } - function recordHintUnsupported(): void { - const previous = defects.get(HINT_UNSUPPORTED_ID); - const count = previous?.class === "hint_unsupported" ? previous.count + 1 : 1; - defects.set(HINT_UNSUPPORTED_ID, { - class: "hint_unsupported", - count, - id: HINT_UNSUPPORTED_ID, - }); - } - function reject(reason: ObservationRejectReason, limitHit: boolean): AcceptOutcome { recordProtocolViolation(reason); if (limitHit) { @@ -638,16 +562,10 @@ export function createObservationRun(input: CreateObservationRunInput): Observat } function snapshot(): ObservationSnapshot { - const connectorSuggestions = [...suggestions.values()]; return Object.freeze({ defects: [...defects.values()].map((defect) => Object.freeze({ ...defect })), facts: facts.map((stored) => Object.freeze({ ...stored.fact, retired: stored.retired })), truncated, - ...(connectorSuggestions.length > 0 - ? { - connector_suggestions: Object.freeze(connectorSuggestions.map((s) => Object.freeze({ ...s }))), - } - : {}), }); } @@ -683,40 +601,11 @@ export function createObservationRun(input: CreateObservationRunInput): Observat }; } - function recordSuggestion(action: string, origin: RecoveryHintOrigin): void { - if (!isPortableAction(action)) { - return; - } - const key = `${origin}:${action}`; - if (!suggestions.has(key)) { - suggestions.set(key, { action, attribution: "connector", origin }); - } - } - - function gateRecoveryHint(action: string, origin: RecoveryHintOrigin): HintGateOutcome { - if (!HINTS_THAT_HOLD.has(action)) { - recordSuggestion(action, origin); - if (input.declared) { - recordHintUnsupported(); - } - return { result: "unsupported" }; - } - if (truncated) { - recordSuggestion(action, origin); - if (input.declared) { - recordHintUnsupported(); - } - return { reason: "truncated", result: "blocked" }; - } - return { result: "authorized" }; - } - return { accept, acceptLine, acceptWire, diagnose, - gateRecoveryHint, hasEmittedId, idLedgerOverflowed: () => idLedgerOverflowed, snapshot, @@ -758,10 +647,6 @@ function findingOf(fact: ConnectorFact, scope: DeclaredScope): Category | undefi } } -function isPortableAction(value: string): value is PortableRecoveryAction { - return PORTABLE_RECOVERY_ACTIONS.has(value); -} - function isRuntimeFactType(value: unknown): boolean { return typeof value === "object" && value !== null && "fact" in value && RUNTIME_FACT_TYPES.has(value.fact); } diff --git a/reference-implementation/server/stores/connector-observation-store.ts b/reference-implementation/server/stores/connector-observation-store.ts index 8737fef3d..24e3cc7be 100644 --- a/reference-implementation/server/stores/connector-observation-store.ts +++ b/reference-implementation/server/stores/connector-observation-store.ts @@ -28,7 +28,6 @@ */ import { execDynamicSqlAcknowledged, iterateDynamicSqlAcknowledged, writeTransaction } from "../../lib/db.ts"; -import { RECOVERY_ACTIONS } from "../../runtime/connector-gap-bounding.ts"; import { type Diagnosis, isObservedFact, @@ -163,8 +162,7 @@ function isSnapshot(value: unknown): value is ObservationSnapshot { isRecord(value) && Array.isArray(value.facts) && Array.isArray(value.defects) && - typeof value.truncated === "boolean" && - (value.connector_suggestions === undefined || isConnectorSuggestions(value.connector_suggestions)) + typeof value.truncated === "boolean" ); } @@ -173,22 +171,6 @@ function hasValidElements(snapshot: ObservationSnapshot): boolean { return snapshot.facts.every(isObservedFact) && snapshot.defects.every(isRuntimeDefect); } -function isConnectorSuggestions(value: unknown): boolean { - return ( - Array.isArray(value) && - value.length <= 16 && - value.every( - (suggestion: unknown) => - isRecord(suggestion) && - Object.keys(suggestion).every((key) => key === "attribution" || key === "action" || key === "origin") && - suggestion.attribution === "connector" && - typeof suggestion.action === "string" && - RECOVERY_ACTIONS.has(suggestion.action) && - (suggestion.origin === "skip_result" || suggestion.origin === "done_error") - ) - ); -} - function isDiagnosis(value: unknown): value is Diagnosis { return ( isRecord(value) && @@ -244,11 +226,11 @@ function encodeRecord(input: ConnectorObservationRunInput): { if (!(isSnapshot(input.snapshot) && isDiagnosis(input.diagnosis))) { throw invalid("connector observation snapshot and diagnosis must match the runtime shapes"); } - const { facts, defects, truncated, connector_suggestions } = input.snapshot; + const { facts, defects, truncated } = input.snapshot; const { category, attribution, basis, contributing } = input.diagnosis; const payload = JSON.stringify({ diagnosis: { attribution, basis, category, contributing }, - snapshot: { defects, facts, truncated, ...(connector_suggestions === undefined ? {} : { connector_suggestions }) }, + snapshot: { defects, facts, truncated }, }); if (Buffer.byteLength(payload, "utf8") > CONNECTOR_OBSERVATION_RECORD_MAX_BYTES) { throw new ConnectorObservationStoreError( diff --git a/reference-implementation/test/connector-observation-store.test.ts b/reference-implementation/test/connector-observation-store.test.ts index 19136ae58..037135a2c 100644 --- a/reference-implementation/test/connector-observation-store.test.ts +++ b/reference-implementation/test/connector-observation-store.test.ts @@ -140,10 +140,7 @@ function observedRun(tag: string): { } return { diagnosis: run.diagnose([`${tag}-auth`, `${tag}-expect`]), - snapshot: { - ...run.snapshot(), - connector_suggestions: [{ action: "refresh_credentials", attribution: "connector", origin: "done_error" }], - }, + snapshot: run.snapshot(), }; } @@ -633,41 +630,6 @@ async function contractValidationAndLimit(harness: Harness): Promise { snapshot: { defects: [], truncated: false }, } as unknown as ConnectorObservationRunInput; await expectStoreError(storeOf().put(missingFacts), "record_invalid", "a snapshot without facts is rejected"); - await inOrder( - [ - { action: "refresh_credentials", attribution: "runtime", origin: "done_error" }, - { action: "arbitrary_action", attribution: "connector", origin: "done_error" }, - { action: "refresh_credentials", attribution: "connector", origin: "arbitrary_origin" }, - { action: "refresh_credentials", attribution: "connector", message: "private text", origin: "done_error" }, - ], - (suggestion) => { - const input = runInput(c, i, "invalid-suggestion"); - return expectStoreError( - storeOf().put({ - ...input, - snapshot: { ...input.snapshot, connector_suggestions: [suggestion] }, - } as unknown as ConnectorObservationRunInput), - "record_invalid", - "only closed connector-attributed suggestion fields may be persisted" - ); - } - ); - const excessiveSuggestions = runInput(c, i, "excessive-suggestions"); - await expectStoreError( - storeOf().put({ - ...excessiveSuggestions, - snapshot: { - ...excessiveSuggestions.snapshot, - connector_suggestions: Array.from({ length: 17 }, () => ({ - action: "refresh_credentials", - attribution: "connector", - origin: "done_error", - })), - }, - } as unknown as ConnectorObservationRunInput), - "record_invalid", - "suggestion storage is bounded by the sixteen portable action and origin pairs" - ); assert.equal(await harness.rowCount(c), CAP, "rejected records do not change the stored rows"); assert.equal((await storeOf().list({ connectorId: c, limit: 100 })).length, CAP, "list limit clamps to 8"); diff --git a/reference-implementation/test/runtime-observations.test.ts b/reference-implementation/test/runtime-observations.test.ts index 6f5d0dd25..f00ba49c6 100644 --- a/reference-implementation/test/runtime-observations.test.ts +++ b/reference-implementation/test/runtime-observations.test.ts @@ -276,14 +276,12 @@ test("invalid traffic does not spend the budget that accepted facts need", () => } assert.equal(run.snapshot().facts.length, 64); assert.equal(run.snapshot().truncated, false, "no valid fact was lost, so the record is not truncated"); - assert.deepEqual(run.gateRecoveryHint("not_retriable", "skip_result"), { result: "authorized" }); }); test("a line that is not JSON never marks the record truncated, however long it is", () => { const { run } = harness(); assert.equal(rejectedReason(run.acceptLine("x".repeat(5000))), "oversize"); assert.equal(run.snapshot().truncated, false); - assert.deepEqual(run.gateRecoveryHint("not_retriable", "skip_result"), { result: "authorized" }); }); test("acceptWire gives the same outcome as acceptLine for a line that is not JSON", () => { @@ -806,93 +804,4 @@ test("diagnosis output carries no action authority", () => { Object.keys(diagnosis).sort((a, b) => (a < b ? -1 : Number(a > b))), ["attribution", "basis", "category", "contributing"] ); - assert.equal(run.gateRecoveryHint("refresh_credentials", "skip_result").result, "unsupported"); -}); - -// --------------------------------------------------------------------------- -// Recovery-hint gate -// --------------------------------------------------------------------------- - -test("not_retriable and unknown hold; other hints are unsupported", () => { - const { run } = harness(); - assert.deepEqual(run.gateRecoveryHint("not_retriable", "skip_result"), { - result: "authorized", - }); - assert.deepEqual(run.gateRecoveryHint("unknown", "skip_result"), { result: "authorized" }); - for (const hint of [ - "refresh_credentials", - "manual_action_required", - "update_selector", - "retry_on_connector_upgrade", - "upstream_unblock", - "retry_by_runtime", - ]) { - assert.deepEqual(run.gateRecoveryHint(hint, "skip_result"), { result: "unsupported" }); - } -}); - -test("an unsupported hint from a declarer adds one bounded hint_unsupported defect and never echoes the action", () => { - const { run } = harness(); - run.gateRecoveryHint("refresh_credentials", "skip_result"); - run.gateRecoveryHint(`SECRET-ACTION-${SECRET}`, "skip_result"); - assert.deepEqual(run.snapshot().defects, [ - { - class: "hint_unsupported", - count: 2, - id: "runtime:connector_defect:hint_unsupported", - }, - ]); - assert.equal(JSON.stringify(run.snapshot()).includes(SECRET), false); -}); - -test("a non-declarer's unsupported hint adds no defect", () => { - const { run } = harness({ declared: false }); - assert.deepEqual(run.gateRecoveryHint("manual_action_required", "skip_result"), { - result: "unsupported", - }); - assert.deepEqual(run.snapshot().defects, []); -}); - -test("a truncated record blocks even a holding hint", () => { - const { run } = harness({ limits: { maxFacts: 64, maxPerWindow: 1000 } }); - for (let i = 0; i < 65; i += 1) { - run.accept(waitExpired(`t${i}`)); - } - assert.equal(run.snapshot().truncated, true); - assert.deepEqual(run.gateRecoveryHint("not_retriable", "skip_result"), { - reason: "truncated", - result: "blocked", - }); - assert.deepEqual(run.gateRecoveryHint("unknown", "skip_result"), { - reason: "truncated", - result: "blocked", - }); - assert.equal( - run.snapshot().defects.find((d) => d.class === "hint_unsupported")?.count, - 2, - "Section 5.11 records unsupported hints even when truncation caused the denial" - ); -}); - -test("connector auth facts never unlock a recovery hint", () => { - const { run } = harness(); - run.accept(authFailure("a1", { rule: "password_rejected" })); - assert.deepEqual(run.diagnose(["a1"]).category, "auth_rejected"); - assert.deepEqual(run.gateRecoveryHint("refresh_credentials", "skip_result"), { - result: "unsupported", - }); -}); - -test("an unauthorized connector hint is recorded once per origin and action as a bounded suggestion", () => { - const { run } = harness(); - assert.equal(run.snapshot().connector_suggestions, undefined); - run.gateRecoveryHint("refresh_credentials", "done_error"); - run.gateRecoveryHint("refresh_credentials", "done_error"); - run.gateRecoveryHint("manual_action_required", "skip_result"); - run.gateRecoveryHint(`SECRET-ACTION-${SECRET}`, "skip_result"); - assert.deepEqual(run.snapshot().connector_suggestions, [ - { action: "refresh_credentials", attribution: "connector", origin: "done_error" }, - { action: "manual_action_required", attribution: "connector", origin: "skip_result" }, - ]); - assert.equal(JSON.stringify(run.snapshot()).includes(SECRET), false); }); From f50e1a2671bcf28a5a6cdab51fa8edcb57424776 Mon Sep 17 00:00:00 2001 From: Tim Nunamaker Date: Fri, 6 Nov 2026 06:31:39 -0600 Subject: [PATCH 8/9] fix(server): check the owner on the run observations read, and anchor the unparseable-line match Gate 3 review of this PR found that GET /_ref/runs/:runId/observations returned any run's record to any owner session that knew the connector id, the instance id and the run id. Nothing tied the instance to the caller. The sibling abandon-enrollment route already returns 404 when the instance belongs to another owner subject. The route now looks the instance up and answers the same typed 404 when the instance is unknown, belongs to another owner subject, or belongs to another connector. This happens before the record is read. The route context gains getOwnerSubjectId and createRequestConnectorInstanceStore, and server/index.ts passes both. Three new tests failed before the change: a second owner subject read the first owner's record. OBSERVATION_TYPE_TEXT_RE matched "type":"OBSERVATION" anywhere in a line that did not parse, so a log line that quoted the pair counted as a discarded OBSERVATION. It now has to start as a JSON object. A prose line is a protocol violation, as any unparseable line is; a truncated OBSERVATION object is still a discarded OBSERVATION. Both cases have a test, and the prose one failed before. findingOf() keeps its behavior: Collection Profile 0.2.0 makes step and rule optional for a provider_message, so a bare auth_failure supports auth_rejected. A comment cites the sections. Signed-off-by: Tim Nunamaker Assisted-by: AI Signed-off-by: Tim Nunamaker --- reference-implementation/runtime/index.ts | 7 ++- .../runtime/observations.ts | 4 ++ reference-implementation/server/index.ts | 2 + .../server/routes/ref-run-observations.ts | 26 ++++++++- .../test/collection-profile-020.test.ts | 44 ++++++++++++++ .../test/ref-run-observations-route.test.ts | 57 ++++++++++++++++++- .../test/runtime-observations.test.ts | 3 + 7 files changed, 138 insertions(+), 5 deletions(-) diff --git a/reference-implementation/runtime/index.ts b/reference-implementation/runtime/index.ts index fbb7fcd86..d086a4587 100644 --- a/reference-implementation/runtime/index.ts +++ b/reference-implementation/runtime/index.ts @@ -2614,8 +2614,11 @@ const RUNTIME_PROTOCOL_CAPABILITIES: readonly ConnectorProtocolCapability[] = [ /** Type of a Collection Profile 0.2.0 OBSERVATION message (Section 5.10). */ const OBSERVATION_MESSAGE_TYPE = "OBSERVATION"; -/** An unparseable line is an OBSERVATION only when its text names the type. */ -const OBSERVATION_TYPE_TEXT_RE = /"type"\s*:\s*"OBSERVATION"/; +/** + * An unparseable line is an OBSERVATION only when it starts as a JSON object and + * its text names the type. Prose that merely quotes the type pair is not one. + */ +const OBSERVATION_TYPE_TEXT_RE = /^\s*\{.*"type"\s*:\s*"OBSERVATION"/s; function isObservationMessage(value: unknown): boolean { return typeof value === "object" && value !== null && (value as { type?: unknown }).type === OBSERVATION_MESSAGE_TYPE; diff --git a/reference-implementation/runtime/observations.ts b/reference-implementation/runtime/observations.ts index 4de74b886..ed6548e6d 100644 --- a/reference-implementation/runtime/observations.ts +++ b/reference-implementation/runtime/observations.ts @@ -625,6 +625,10 @@ function findingOf(fact: ConnectorFact, scope: DeclaredScope): Category | undefi if (fact.attrs.kind !== "auth_failure") { return undefined; } + // `step` is OPTIONAL for this fact and so is `rule` (docs/spec/collection-profile.md at + // data-connectors a87edcdc, :1525 and :1541), and an `auth_failure` with no runtime + // record derives `auth_rejected` as the connector's report (:1702). So a bare one + // supports the category. A step or rule that it does name must be declared. if (fact.step !== undefined && !scope.steps.has(fact.step)) { return undefined; } diff --git a/reference-implementation/server/index.ts b/reference-implementation/server/index.ts index bffb7fe79..43c5ace67 100644 --- a/reference-implementation/server/index.ts +++ b/reference-implementation/server/index.ts @@ -5956,6 +5956,8 @@ export function buildAsApp(opts: ServerOpts = {}) { } as unknown as Parameters[1]); // Owner-only diagnostic read of a run's OBSERVATION record (Collection Profile 0.2.0 Section 5.10). mountRefRunObservations(app, { + createRequestConnectorInstanceStore, + getOwnerSubjectId, handleError, pdppError, requireOwnerSession: ownerAuth.requireOwnerSession, diff --git a/reference-implementation/server/routes/ref-run-observations.ts b/reference-implementation/server/routes/ref-run-observations.ts index c2c3f0257..fad6b8f14 100644 --- a/reference-implementation/server/routes/ref-run-observations.ts +++ b/reference-implementation/server/routes/ref-run-observations.ts @@ -8,8 +8,12 @@ // // The record is keyed by connector, connector instance and run. A run id that // exists under another connector or instance is absent, so it returns a typed -// `not_found` 404. This route exports nothing: it returns the stored record to -// the owner session that asked, and nothing else reads it. +// `not_found` 404. The instance must also belong to the calling owner subject, +// as in the `abandon-enrollment` route of ref-browser-enrollment-shell.ts: an +// unknown instance, another owner's instance and an instance of another +// connector all give the same 404, before the record is read. This route +// exports nothing: it returns the stored record to the owner session that +// asked, and nothing else reads it. import type { ConnectorObservationRunStore } from "../stores/connector-observation-store.ts"; import { @@ -34,7 +38,18 @@ interface AppLike { get: (path: string, ...handlers: (MiddlewareHandler | RouteHandler)[]) => AppLike; } +interface OwnedConnectorInstance { + readonly connectorId: string; + readonly ownerSubjectId: string; +} + +interface ConnectorInstanceLookup { + get: (id: string) => Promise | OwnedConnectorInstance | null; +} + export interface MountRefRunObservationsContext { + createRequestConnectorInstanceStore: () => ConnectorInstanceLookup; + getOwnerSubjectId: (req: unknown) => string; handleError: (res: unknown, err: unknown) => void; pdppError: PdppErrorFn; requireOwnerSession: MiddlewareHandler; @@ -87,6 +102,13 @@ export function mountRefRunObservations(app: AppLike, ctx: MountRefRunObservatio } } + // Ownership first. The three misses (unknown, foreign, other connector) are indistinguishable. + const ownerSubjectId = ctx.getOwnerSubjectId(req); + const instance = await ctx.createRequestConnectorInstanceStore().get(connectorInstanceId); + if (!instance || instance.ownerSubjectId !== ownerSubjectId || instance.connectorId !== connectorId) { + return ctx.pdppError(res, 404, "not_found", `Run observations not found: ${runId}`, "run_id"); + } + const store = ctx.store ?? getDefaultConnectorObservationStore(); const record = await store.get({ connectorId, connectorInstanceId, runId }); if (record === null) { diff --git a/reference-implementation/test/collection-profile-020.test.ts b/reference-implementation/test/collection-profile-020.test.ts index 26f52b837..6b5a8dac2 100644 --- a/reference-implementation/test/collection-profile-020.test.ts +++ b/reference-implementation/test/collection-profile-020.test.ts @@ -423,6 +423,50 @@ test("a malformed line never reaches the trace or the run error, whatever its ty } }); +test("prose that quotes the OBSERVATION type is not an OBSERVATION: only a line that starts as a JSON object is", async () => { + // OBSERVATION_TYPE_TEXT_RE classifies a line that does not parse. A log line that + // merely quotes the type pair is still a protocol violation (as any unparseable + // line is), not a malformed OBSERVATION that is discarded while the run goes on. + const stub = makeStub( + [ + 'note: the connector should emit {"type":"OBSERVATION"} here', + { records_emitted: 0, status: "succeeded", type: "DONE" }, + ], + 0 + ); + try { + const outcome = await runStub(stub, "run_c020_prose_quotes_type", manifest()); + assert.equal(outcome.ok, false, "a prose line is a protocol violation, as any unparseable line is"); + const record = await persistedRecord("run_c020_prose_quotes_type"); + assert.deepEqual( + (record?.snapshot.defects ?? []).map(defectKey), + [], + "a prose line must not accrue an OBSERVATION defect" + ); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } +}); + +test("a truncated OBSERVATION object is a discarded OBSERVATION and collection continues", async () => { + const stub = makeStub( + ['{"type":"OBSERVATION","id":"cut1"', { records_emitted: 0, status: "succeeded", type: "DONE" }], + 0 + ); + try { + const outcome = await runStub(stub, "run_c020_truncated_observation", manifest()); + assert.equal( + outcome.ok, + true, + `a truncated OBSERVATION must not abort the run: ${outcome.ok ? "" : outcome.message}` + ); + const record = await persistedRecord("run_c020_truncated_observation"); + assert.deepEqual((record?.snapshot.defects ?? []).map(defectKey), ["invalid"]); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } +}); + test("an oversize line that is not JSON records oversize without marking the record truncated", async () => { // This goes through the runtime's child stdout path, not the unit entry. const stub = makeStub( diff --git a/reference-implementation/test/ref-run-observations-route.test.ts b/reference-implementation/test/ref-run-observations-route.test.ts index ca5207a5c..25197aeca 100644 --- a/reference-implementation/test/ref-run-observations-route.test.ts +++ b/reference-implementation/test/ref-run-observations-route.test.ts @@ -23,6 +23,20 @@ const RUN_ID = "run_observations_route"; type Session = "owner" | "client" | "anonymous"; +const OWNER_SUBJECT = "owner_subject_1"; +const OTHER_OWNER_SUBJECT = "owner_subject_2"; + +interface ViewerOptions { + /** The owner subject of the calling session. */ + readonly subject?: string; + /** Instances the connector-instance store knows, by id. Defaults to one instance owned by OWNER_SUBJECT. */ + readonly instances?: Readonly>; +} + +const DEFAULT_INSTANCES: NonNullable = { + [INSTANCE_ID]: { connectorId: CONNECTOR_ID, ownerSubjectId: OWNER_SUBJECT }, +}; + interface Captured { body: Record | null; headers: Record; @@ -43,6 +57,7 @@ function fakeRecord(): ConnectorObservationRunRecord { function fakeStore(record: ConnectorObservationRunRecord | null): ConnectorObservationRunStore { const calls: unknown[] = []; return { + calls, get(key: ConnectorObservationRunKey) { calls.push(key); if ( @@ -70,8 +85,10 @@ async function invoke( session: Session, query: Record, store: ConnectorObservationRunStore, - runId = RUN_ID + runId = RUN_ID, + viewer: ViewerOptions = {} ): Promise { + const instances = viewer.instances ?? DEFAULT_INSTANCES; type Handler = (req: unknown, res: unknown, next?: () => void) => unknown; const routes: { middleware: Handler[]; handler: Handler | undefined } = { handler: undefined, middleware: [] }; const app = { @@ -83,6 +100,15 @@ async function invoke( }, }; mountRefRunObservations(app as never, { + createRequestConnectorInstanceStore: () => ({ + get: (id: string) => { + const found = instances[id]; + return found + ? { connectorId: found.connectorId, connectorInstanceId: id, ownerSubjectId: found.ownerSubjectId } + : null; + }, + }), + getOwnerSubjectId: () => viewer.subject ?? OWNER_SUBJECT, handleError: (_res, err) => { throw err; }, @@ -176,6 +202,35 @@ test("a run that exists under another connector or instance is not found", async assert.equal(otherInstance.status, 404); }); +test("a second owner subject gets 404 for the first owner's run, and the store is never read", async () => { + const store = fakeStore(fakeRecord()); + const result = await invoke("owner", OWNER_QUERY, store, RUN_ID, { subject: OTHER_OWNER_SUBJECT }); + assert.equal(result.status, 404); + assert.equal((result.body?.error as { code?: string } | undefined)?.code, "not_found"); + assert.notEqual(result.body?.object, "run_observations"); + assert.equal(result.body?.snapshot, undefined, "the foreign owner must not see any part of the record"); + assert.equal( + (store as unknown as { calls: unknown[] }).calls.length, + 0, + "ownership is decided before the record is read" + ); +}); + +test("an instance the store does not know is a 404 even when a record exists under that key", async () => { + const result = await invoke("owner", OWNER_QUERY, fakeStore(fakeRecord()), RUN_ID, { instances: {} }); + assert.equal(result.status, 404); + assert.equal((result.body?.error as { code?: string } | undefined)?.code, "not_found"); +}); + +test("an instance registered under a different connector is a 404", async () => { + const result = await invoke("owner", OWNER_QUERY, fakeStore(fakeRecord()), RUN_ID, { + instances: { + [INSTANCE_ID]: { connectorId: "https://registry.pdpp.dev/connectors/other", ownerSubjectId: OWNER_SUBJECT }, + }, + }); + assert.equal(result.status, 404); +}); + test("an unknown run id is a typed not_found 404", async () => { const result = await invoke("owner", OWNER_QUERY, fakeStore(null), "run_missing"); assert.equal(result.status, 404); diff --git a/reference-implementation/test/runtime-observations.test.ts b/reference-implementation/test/runtime-observations.test.ts index f00ba49c6..0afa491c6 100644 --- a/reference-implementation/test/runtime-observations.test.ts +++ b/reference-implementation/test/runtime-observations.test.ts @@ -717,6 +717,9 @@ test("violation defects aggregate by reason with counts and no connector text", // Diagnosis // --------------------------------------------------------------------------- +// Section 5.10.1: `step` and `rule` are OPTIONAL for provider_message, so a bare +// auth_failure (no step, no rule) supports auth_rejected. A named step or rule must be declared +// (see the test further down). test("an auth_failure cited in the basis diagnoses auth_rejected attributed to the connector", () => { const { run } = harness(); run.accept(authFailure("a1")); From fe63a4bc880e5ccd325878179c82fcccb8d630dd Mon Sep 17 00:00:00 2001 From: Tim Nunamaker Date: Fri, 6 Nov 2026 11:01:12 -0600 Subject: [PATCH 9/9] fix(runtime): record OBSERVATION diagnostics only for runs that have something to record persistObservationDiagnosis ran on every terminal path, so every run of every connector, legacy ones included, wrote a connector_observation_runs row. On PostgreSQL the terminal result waits for that write, up to the writer-lock bound. The rows also share the per-connector cap of eight, so empty rows pushed out the one row that explained a failure. The write now happens only when the run's snapshot holds an accepted fact or a defect. The owner-cancel seal still happens at the first settle point. A run with nothing to record has no row; the read route already answers 404 for a missing record. The new test ran a run with a fact, then more silent runs than the cap holds, legacy and declared-but-silent. It failed before: the silent runs had rows and the informative one was evicted. Assisted-by: AI Signed-off-by: Tim Nunamaker --- reference-implementation/runtime/index.ts | 26 ++++++++--- .../test/collection-profile-020.test.ts | 44 ++++++++++++++++++- 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/reference-implementation/runtime/index.ts b/reference-implementation/runtime/index.ts index d086a4587..235106c22 100644 --- a/reference-implementation/runtime/index.ts +++ b/reference-implementation/runtime/index.ts @@ -106,7 +106,7 @@ import { } from "./done-validators.ts"; import type { IngestResult as StrictIngestResult } from "./ingest-failure.ts"; import { readIngestResponse as strictReadIngestResponse } from "./ingest-failure.ts"; -import { createObservationRun, type WireParse } from "./observations.ts"; +import { createObservationRun, type ObservationSnapshot, type WireParse } from "./observations.ts"; import { buildHttpFailure, buildIngestEnvelopeContractViolationFailure, @@ -6563,8 +6563,8 @@ export async function runConnector(opts: RuntimeRunConnectorOptions): Promise { + return getDefaultConnectorObservationStore() .put({ connectorId, connectorInstanceId: resolvedConnectorInstanceId, // A cancelled run ended by the owner's choice, not by the connector's cause. diagnosis: observationRun.diagnose(ownerCancelRequested ? [] : doneBasis), runId, - snapshot: observationRun.snapshot(), + snapshot, }) .then( () => undefined, @@ -6594,7 +6609,6 @@ export async function runConnector(opts: RuntimeRunConnectorOptions): Promise { diff --git a/reference-implementation/test/collection-profile-020.test.ts b/reference-implementation/test/collection-profile-020.test.ts index 6b5a8dac2..d1504adb7 100644 --- a/reference-implementation/test/collection-profile-020.test.ts +++ b/reference-implementation/test/collection-profile-020.test.ts @@ -21,7 +21,10 @@ import { closeDb, getDb, initDb } from "../server/db.ts"; import { closePostgresStorage, initPostgresStorage, postgresQuery } from "../server/postgres-storage.ts"; import { createRequestConnectorInstanceStore } from "../server/request-store-factories.ts"; import { admitOwnerRunConnection } from "../server/stores/connector-instance-store.ts"; -import { getDefaultConnectorObservationStore } from "../server/stores/connector-observation-store.ts"; +import { + CONNECTOR_OBSERVATION_RUNS_PER_CONNECTOR, + getDefaultConnectorObservationStore, +} from "../server/stores/connector-observation-store.ts"; import { dedicatedPostgresTestUrl } from "./helpers/dedicated-postgres-test-url.ts"; const CONNECTOR_ID = "collection-profile-020-test"; @@ -825,6 +828,45 @@ test("a valid 0.1.0 run without OBSERVATION still succeeds", async () => { } }); +test("a run with no OBSERVATION and no defects writes no row, so it cannot evict an informative one", async () => { + const informativeId = "run_c020_cap_informative"; + const informative = makeStub([PROVIDER_REJECTED("p1"), { records_emitted: 0, status: "succeeded", type: "DONE" }], 0); + try { + const first = await runStub(informative, informativeId, manifest()); + assert.equal(first.ok, true, `the informative run must succeed: ${first.ok ? "" : first.message}`); + assert.ok(await persistedRecord(informativeId), "a run with an accepted fact still writes its row"); + } finally { + rmSync(informative.dir, { force: true, recursive: true }); + } + // More silent runs than the per-connector cap holds: legacy connectors, and a + // 0.2.0 connector that declared OBSERVATION and sent none. + const silentRunIds: string[] = []; + for (let index = 0; index < CONNECTOR_OBSERVATION_RUNS_PER_CONNECTOR + 2; index += 1) { + const stub = makeStub([{ records_emitted: 0, status: "succeeded", type: "DONE" }], 0); + const runId = `run_c020_cap_silent_${String(index)}`; + silentRunIds.push(runId); + try { + const outcome = await runStub( + stub, + runId, + index % 2 === 0 + ? manifest({ diagnostic_descriptors: undefined, protocol_capabilities: undefined, protocol_version: "0.1.0" }) + : manifest() + ); + assert.equal(outcome.ok, true, `a silent run must succeed: ${outcome.ok ? "" : outcome.message}`); + } finally { + rmSync(stub.dir, { force: true, recursive: true }); + } + } + for (const runId of silentRunIds) { + assert.equal(await persistedRecord(runId), null, `${runId}: no OBSERVATION and no defects, so no row`); + } + assert.ok( + await persistedRecord(informativeId), + "the informative row is still there after more silent runs than the cap holds" + ); +}); + function existsMarker(path: string): boolean { try { readFileSync(path);