diff --git a/.changeset/schema-diff-unreachable-kind.md b/.changeset/schema-diff-unreachable-kind.md new file mode 100644 index 0000000000..2c51e42f1a --- /dev/null +++ b/.changeset/schema-diff-unreachable-kind.md @@ -0,0 +1,15 @@ +--- +"@objectstack/spec": minor +"@objectstack/service-datasource": patch +"@objectstack/runtime": patch +--- + +**Federation:** `SchemaDiffEntry` gains a distinct `unreachable` kind — "the remote could not be read" is no longer reported as `missing_table`, and a transient outage no longer aborts boot under the default `onMismatch: 'fail'` (#11166, maintainer ruling 2026-08-23). + +`ExternalDatasourceService.validateEach` used to convert **any** per-object validation throw — including `connect ECONNREFUSED` from remote introspection — into a `{ kind: 'missing_table', severity: 'error' }` row, indistinguishable from a genuinely dropped table. Downstream, that shape meant: the boot gate (`ExternalValidationPlugin.runValidation`) aborted startup for a 30-second network blip, and the background drift checker raised `external.schema.drift` events claiming the schema changed on every tick the remote stayed down. + +Now: + +- **`@objectstack/spec`** (minor): `SchemaDiffEntryKind` adds `'unreachable'` — the one kind that asserts *nothing about the remote schema*; it states that validation was indeterminate because the remote (or the object definition) could not be read. The throwing error's text is carried in `actual`. Every other kind remains a measured fact about a schema that was successfully read. Additive: existing entries and their meanings are unchanged. Consumers that exhaustively switch on the kind union (e.g. a `Record`) will get a compile-time prompt to label the new member; non-exhaustive consumers see a new string value at runtime and should render it as-is. +- **`@objectstack/service-datasource`** (patch): the per-object catch in `validateEach` classifies every throw as `unreachable` (rows stay `ok: false`, `severity: 'error'`). `missing_table` is still reported — but only from its measured branch: a table absent from an introspection that returned. +- **`@objectstack/runtime`** (patch): the boot gate no longer feeds `unreachable` rows to the `onMismatch` policy — no abort under `fail`; instead it logs a loud `warn` naming the datasource, the object, the underlying error, and that the object's schema is unverified for this boot, under every `onMismatch` value. Measured mismatches keep the existing policy behavior, including sitting beside an unreachable row in the same report. The drift checker still emits `external.schema.drift` for unreachable rows (consumers discriminate on `kind`), but its operator-facing summary now says "could not read the remote", never "drift detected", for them. diff --git a/packages/runtime/src/external-validation-plugin.test.ts b/packages/runtime/src/external-validation-plugin.test.ts index 04184a75a8..9305c697cb 100644 --- a/packages/runtime/src/external-validation-plugin.test.ts +++ b/packages/runtime/src/external-validation-plugin.test.ts @@ -28,6 +28,11 @@ const sampleDiffs: SchemaDiffEntry[] = [ { kind: 'type_mismatch', remoteName: 'fact_orders', column: 'amount', expected: 'number', actual: 'text', severity: 'error' }, ]; +/** [#11166] The row `validateEach` produces when the remote could not be read. */ +const unreachableDiffs: SchemaDiffEntry[] = [ + { kind: 'unreachable', remoteName: 'fact_orders', actual: 'connect ECONNREFUSED 10.0.0.5:5432', severity: 'error' }, +]; + describe('ExternalValidationPlugin (ADR-0015 Gate 2)', () => { it('subscribes to kernel:ready in start()', () => { const { ctx } = makeCtx({}); @@ -80,6 +85,65 @@ describe('ExternalValidationPlugin (ADR-0015 Gate 2)', () => { }); await expect(new ExternalValidationPlugin().runValidation(ctx)).rejects.toBeInstanceOf(ExternalSchemaMismatchError); }); + + /** + * [#11166] An `unreachable` row is not a schema mismatch: validation was + * indeterminate (the remote could not be read), so the default + * `onMismatch: 'fail'` must NOT abort boot for it — a transient outage + * during startup used to be a refusal to start. Loud logging instead, + * per the maintainer ruling of 2026-08-23. + */ + it('does not abort boot for an `unreachable` row under onMismatch=fail — logs loudly instead (#11166)', async () => { + const { ctx, warnings, infos } = makeCtx({ + 'external-datasource': { + validateAll: async () => ({ + ok: false, + results: [{ ok: false, datasource: 'warehouse', object: 'wh_order', diffs: unreachableDiffs }], + }), + }, + metadata: { get: async () => ({ schemaMode: 'external', external: { validation: { onMismatch: 'fail' } } }) }, + }); + await expect(new ExternalValidationPlugin().runValidation(ctx)).resolves.toBeUndefined(); + const line = warnings.find((w) => String(w[0]).includes('could not be validated')); + expect(line).toBeDefined(); + // The log names the datasource, the object, and the underlying error. + expect(line![1]).toMatchObject({ + datasource: 'warehouse', + object: 'wh_order', + errors: ['connect ECONNREFUSED 10.0.0.5:5432'], + }); + // And it is NOT the all-clear: an unverified boot must not read as clean. + expect(infos.some((i) => String(i[0]).includes('match their remote schema'))).toBe(false); + }); + + it('still aborts for a measured mismatch even when another row is merely unreachable (#11166)', async () => { + const { ctx } = makeCtx({ + 'external-datasource': { + validateAll: async () => ({ + ok: false, + results: [ + { ok: false, datasource: 'warehouse', object: 'wh_down', diffs: unreachableDiffs }, + { ok: false, datasource: 'warehouse', object: 'wh_order', diffs: sampleDiffs }, + ], + }), + }, + }); + await expect(new ExternalValidationPlugin().runValidation(ctx)).rejects.toBeInstanceOf(ExternalSchemaMismatchError); + }); + + it('logs the unreachable row even under onMismatch=ignore — an outage is not a mismatch the policy covers (#11166)', async () => { + const { ctx, warnings } = makeCtx({ + 'external-datasource': { + validateAll: async () => ({ + ok: false, + results: [{ ok: false, datasource: 'warehouse', object: 'wh_order', diffs: unreachableDiffs }], + }), + }, + metadata: { get: async () => ({ schemaMode: 'external', external: { validation: { onMismatch: 'ignore' } } }) }, + }); + await expect(new ExternalValidationPlugin().runValidation(ctx)).resolves.toBeUndefined(); + expect(warnings.some((w) => String(w[0]).includes('could not be validated'))).toBe(true); + }); }); describe('ExternalValidationPlugin — background drift detection (ADR-0015 §5.2)', () => { @@ -123,6 +187,30 @@ describe('ExternalValidationPlugin — background drift detection (ADR-0015 §5. }); }); + /** + * [#11166] A briefly-unreachable remote used to raise `external.schema.drift` + * events whose diffs claimed `missing_table` on every tick it stayed down. + * The event is still emitted (audit/notification consumers see the outage) + * but under the distinct `unreachable` kind — and the operator-facing + * summary log says "could not read", never "drift detected". + */ + it('runDriftCheck reports an unreachable remote under the `unreachable` kind, not as drift (#11166)', async () => { + const { ctx, warnings } = makeCtx({ + 'external-datasource': scopedService([ + { ok: false, datasource: 'warehouse', object: 'wh_order', diffs: unreachableDiffs }, + ]), + }); + const emitted = await new ExternalValidationPlugin().runDriftCheck(ctx, 'warehouse'); + expect(emitted).toBe(1); + expect(ctx.trigger).toHaveBeenCalledWith('external.schema.drift', { + datasource: 'warehouse', + object: 'wh_order', + diffs: unreachableDiffs, + }); + expect(warnings.some((w) => String(w[0]).includes('could not read the remote'))).toBe(true); + expect(warnings.some((w) => String(w[0]).includes('drift detected'))).toBe(false); + }); + it('runDriftCheck is a no-op (no throw) when the scoped validation rejects', async () => { const { ctx, warnings } = makeCtx({ 'external-datasource': { diff --git a/packages/runtime/src/external-validation-plugin.ts b/packages/runtime/src/external-validation-plugin.ts index a77fbf6d3e..98ccac8bf6 100644 --- a/packages/runtime/src/external-validation-plugin.ts +++ b/packages/runtime/src/external-validation-plugin.ts @@ -180,6 +180,12 @@ export interface ExternalSchemaDriftEvent { * - `warn` → logs the diff and continues, * - `ignore` → does nothing. * + * `onMismatch` governs MEASURED mismatches only. A row whose diffs are + * `kind: 'unreachable'` (the remote could not be read, so validation was + * indeterminate — see the kind's docblock in `@objectstack/spec/shared`) never + * feeds that policy: it is logged loudly and boot continues, under every + * `onMismatch` value (maintainer ruling 2026-08-23, #11166). + * * No-op when the `external-datasource` service is not registered (federation * unused). */ @@ -273,18 +279,49 @@ export class ExternalValidationPlugin implements Plugin { } for (const r of failures) { + // [#11166] An `unreachable` row is NOT a schema mismatch — the remote + // (or the object's own definition) could not be read, so validation was + // indeterminate and there is no measured fact to gate on. Maintainer + // ruling 2026-08-23: no `onMismatch: 'fail'` abort for unreachable — + // loud logging instead. The log is deliberately OUTSIDE the `onMismatch` + // resolution: that policy governs how a measured mismatch is handled, + // and an outage is a different condition — even an `ignore` datasource's + // operator is told their boot ran unverified. `warn`, not `error`, per + // AGENTS.md's degradation table: this is a functional degradation (a + // check did not run, and says so) — nothing claims to have persisted. + const unreachable = r.diffs.filter((d) => d.kind === 'unreachable'); + const schemaDiffs = r.diffs.filter((d) => d.kind !== 'unreachable'); + if (unreachable.length > 0) { + ctx.logger?.warn?.( + '[external-validation] federated object could not be validated — the remote (or the ' + + 'object definition) could not be read, so its schema is UNVERIFIED for this boot: no ' + + 'mismatch abort applies because no mismatch was measured, and boot continues. The ' + + 'remote table may be perfectly intact; do not "repair" a schema nobody has seen. Fix: ' + + 'check the datasource connection/credentials, then re-run validation (restart, ' + + '`os datasource validate`, or the background drift check).', + { + datasource: r.datasource, + object: r.object, + errors: unreachable.map((d) => d.actual ?? '(no error text)'), + }, + ); + } + // A row today carries either measured diffs or one unreachable row, but + // judge on what is present rather than on the producer's current shape: + // only MEASURED diffs reach the onMismatch policy. + if (schemaDiffs.length === 0) continue; const mode = await resolveOnMismatch(metadata, r.datasource); if (mode === 'ignore') continue; if (mode === 'warn') { ctx.logger?.warn?.('[external-validation] external schema drift', { datasource: r.datasource, object: r.object, - diffs: r.diffs, + diffs: schemaDiffs, }); continue; } // mode === 'fail' (default) - throw new ExternalSchemaMismatchError(r.datasource, r.object, r.diffs); + throw new ExternalSchemaMismatchError(r.datasource, r.object, schemaDiffs); } } @@ -447,10 +484,27 @@ export class ExternalValidationPlugin implements Plugin { } } if (drifted.length > 0) { - ctx.logger?.warn?.('[external-validation] background drift detected', { - datasource, - objects: drifted.map((r) => r.object), - }); + // [#11166] Same distinction as the boot gate, one layer down: an + // `unreachable` row is "could not watch", not "schema changed". The + // event above is still emitted for it — audit/notification consumers + // discriminate on the entry's `kind` — but the operator-facing summary + // must not claim drift for a remote nobody read. + const changed = drifted.filter((r) => r.diffs.some((d) => d.kind !== 'unreachable')); + const unread = drifted.filter((r) => r.diffs.every((d) => d.kind === 'unreachable')); + if (changed.length > 0) { + ctx.logger?.warn?.('[external-validation] background drift detected', { + datasource, + objects: changed.map((r) => r.object), + }); + } + if (unread.length > 0) { + ctx.logger?.warn?.( + '[external-validation] background drift check could not read the remote — these objects ' + + 'are UNWATCHED this tick, not drifted; the schema was not seen. Fix: check the ' + + 'datasource connection/credentials.', + { datasource, objects: unread.map((r) => r.object) }, + ); + } } return drifted.length; } diff --git a/packages/services/service-datasource/src/__tests__/external-datasource-service.test.ts b/packages/services/service-datasource/src/__tests__/external-datasource-service.test.ts index 3a17d834b2..378b566fbe 100644 --- a/packages/services/service-datasource/src/__tests__/external-datasource-service.test.ts +++ b/packages/services/service-datasource/src/__tests__/external-datasource-service.test.ts @@ -439,6 +439,78 @@ describe('validateAll', () => { expect(report.ok).toBe(true); expect(report.results.map((r) => r.object)).toEqual(['wh_order']); }); + + /** + * [#11166] The card's measured defect, reproduced: an introspector throwing + * `connect ECONNREFUSED 10.0.0.5:5432` used to come back as + * `kind: 'missing_table'` — indistinguishable from a genuinely dropped + * table, and an abort under the boot gate's default `onMismatch: 'fail'`. + * Maintainer ruling 2026-08-23: a throw is classified as the distinct + * `unreachable` kind. The row stays non-`ok` (silently `ok: true` for an + * object nobody could validate would be the invent-an-answer failure), and + * the connection error's text is carried in `actual`. + */ + it('classifies a per-object throw as `unreachable`, never `missing_table` (#11166)', async () => { + const objects: ObjectLike[] = [ + { + name: 'wh_a_orders', + datasource: 'wh_a', + external: { remoteName: 'orders' }, + fields: { order_id: { type: 'text' } }, + }, + ]; + const svc = new ExternalDatasourceService({ + introspect: async () => { + throw new Error('connect ECONNREFUSED 10.0.0.5:5432'); + }, + getDatasource: async () => ({ name: 'wh_a', schemaMode: 'external' }), + getObject: async (n) => objects.find((o) => o.name === n), + listObjects: async () => objects, + logger: { warn: () => {} }, + }); + + const report = await svc.validateAll(); + + expect(report.ok).toBe(false); + expect(report.results).toEqual([ + { + ok: false, + datasource: 'wh_a', + object: 'wh_a_orders', + diffs: [ + { + kind: 'unreachable', + remoteName: 'orders', + actual: 'connect ECONNREFUSED 10.0.0.5:5432', + severity: 'error', + }, + ], + }, + ]); + }); + + /** + * [#11166] The other half of the distinction: a table genuinely absent from + * a schema that WAS read stays `missing_table` — the classification change + * must not blur the measured fact into the indeterminate kind. + */ + it('still reports `missing_table` for a table absent from a successfully read schema', async () => { + const svc = makeService({ + objects: [ + { + name: 'wh_order', + datasource: 'warehouse', + external: { remoteName: 'ghost' }, + fields: { order_id: { type: 'text' } }, + }, + ], + }); + const report = await svc.validateAll(); + expect(report.ok).toBe(false); + expect(report.results[0].diffs).toEqual([ + expect.objectContaining({ kind: 'missing_table', severity: 'error' }), + ]); + }); }); /** @@ -577,6 +649,8 @@ describe('validateDatasource', () => { // The scoped path still turns a per-object throw into a row rather than // rejecting the whole report — the sweep's `catch`, not a second one. + // [#11166] The row's kind is `unreachable` (the fixture's introspector + // threw a connection error), no longer the invented `missing_table`. expect(introspected).toEqual(['wh_b']); expect(report.ok).toBe(false); expect(report.results).toEqual((await sweptThenFiltered('wh_b', opts)).results); @@ -584,7 +658,7 @@ describe('validateDatasource', () => { ok: false, datasource: 'wh_b', object: 'wh_b_orders', - diffs: [expect.objectContaining({ kind: 'missing_table', severity: 'error' })], + diffs: [expect.objectContaining({ kind: 'unreachable', severity: 'error' })], }); }); diff --git a/packages/services/service-datasource/src/external-datasource-service.ts b/packages/services/service-datasource/src/external-datasource-service.ts index a88f4e6402..0a91dcc315 100644 --- a/packages/services/service-datasource/src/external-datasource-service.ts +++ b/packages/services/service-datasource/src/external-datasource-service.ts @@ -687,10 +687,36 @@ export class ExternalDatasourceService implements IExternalDatasourceService { /** * Validate a chosen set of objects, one report. * - * A per-object throw becomes a `missing_table` row carrying the thrower's + * A per-object throw becomes an `unreachable` row carrying the thrower's * message rather than rejecting the whole report: one unreachable remote (or * one object whose definition vanished mid-sweep) must not erase the verdicts * of the objects that did validate. + * + * ## Why the row's kind is `unreachable` for EVERY throw — no error sniffing + * + * This catch used to invent `kind: 'missing_table'`, so a refused + * connection, a DNS failure, an auth expiry or a timeout out of + * `introspect(datasource)` was indistinguishable from a genuinely dropped + * remote table — and the boot gate's default `onMismatch: 'fail'` turned a + * 30-second network blip into a refusal to start (maintainer ruling + * 2026-08-23: an unreachable remote is not a schema mismatch). + * + * The discrimination "connection failure or schema fact?" is STRUCTURAL + * here, not an error-signature question. Every schema FACT this service + * reports (`missing_table` included) is derived from an introspection that + * **returned** — `validateObject`'s `!table` branch asserts `missing_table` + * from a successfully read schema in which the table is absent. A throw + * means the comparison never ran, and per the repo's read-failure + * classification precedent (`READ_FAILURE_DISCRIMINATORS`, + * `packages/metadata/src/utils/schema-sync-errors.ts`: a fact verdict must + * be POSITIVELY EARNED, never defaulted to), no signature test on the thrown + * value can earn a claim about a remote schema nobody read. Deliberately NOT + * a hand-rolled `err.code` allowlist — an unrecognised connection error + * would fall back to the wrong fact — and deliberately not a + * missing-table-shaped rescue either: on this path a "no such table" error + * would be about the METADATA store or the introspection machinery, not the + * remote table this row names, so rescuing `missing_table` from it would + * mislabel in a second direction. */ private async validateEach(objects: ObjectLike[]): Promise { const results = await Promise.all( @@ -703,9 +729,14 @@ export class ExternalDatasourceService implements IExternalDatasourceService { object: o.name, diffs: [ { - kind: 'missing_table', + kind: 'unreachable', remoteName: o.external?.remoteName ?? o.name, actual: err instanceof Error ? err.message : String(err), + // 'error', not 'warning': the object is NOT verified, `ok` + // must stay false, and interactive consumers (CLI validate, + // Studio) should present it at attention level. The + // transient-vs-fact distinction consumers act on is the KIND + // axis, not severity (see the kind's docblock in spec). severity: 'error', }, ], diff --git a/packages/spec/src/shared/external-errors.test.ts b/packages/spec/src/shared/external-errors.test.ts index 1d7ff69279..08ab908bf8 100644 --- a/packages/spec/src/shared/external-errors.test.ts +++ b/packages/spec/src/shared/external-errors.test.ts @@ -105,6 +105,27 @@ describe('renderDiffMessage', () => { expect(lines[2]).toContain('expected datetime'); expect(lines[2]).toContain('actual text'); }); + + /** + * [#11166] `unreachable` is a member of the kind vocabulary (the "could not + * be read" entry — a statement that validation was indeterminate, not a + * schema fact) and renders like every other kind: the raw kind name plus the + * carried error text, so an unknown-to-a-consumer entry is still loud. + */ + it('renders the `unreachable` kind with the carried error text (#11166)', () => { + const diffs: SchemaDiffEntry[] = [ + { + kind: 'unreachable', + remoteName: 'fact_orders', + actual: 'connect ECONNREFUSED 10.0.0.5:5432', + severity: 'error', + }, + ]; + const lines = renderDiffMessage('warehouse', 'wh_order', diffs).split('\n'); + expect(lines).toHaveLength(2); + expect(lines[1]).toContain('unreachable'); + expect(lines[1]).toContain('connect ECONNREFUSED 10.0.0.5:5432'); + }); }); describe('ExternalSchemaMismatchError', () => { diff --git a/packages/spec/src/shared/external-errors.ts b/packages/spec/src/shared/external-errors.ts index 3099c521ba..33e3900b92 100644 --- a/packages/spec/src/shared/external-errors.ts +++ b/packages/spec/src/shared/external-errors.ts @@ -113,7 +113,27 @@ export type SchemaDiffEntryKind = * rejected write: a `defaultValue` runtime token emitted as a literal DEFAULT * stamped the token's own spelling into every omitted insert (#4560). */ - | 'default_mismatch'; + | 'default_mismatch' + /** + * The validator COULD NOT READ what the comparison needs — remote schema + * introspection failed (connection refused, DNS, expired credentials, a + * timeout), or the object/datasource definition vanished mid-sweep — so the + * comparison never ran. + * + * Unlike every other kind, this entry asserts **nothing about the remote + * schema**: it is "validation was indeterminate", a condition that is often + * TRANSIENT (the remote table may be perfectly intact and the next attempt + * may succeed), where the other kinds each state a measured FACT about a + * schema that was successfully read. Consumers must keep the two apart: + * treating an unreachable remote as a mismatch tells an operator (or an AI) + * to "repair" a schema nobody has seen — e.g. recreate a table that was + * never dropped. The boot gate logs it loudly and continues rather than + * applying `onMismatch`, and drift consumers should surface it as "cannot + * watch", never as "schema changed" (maintainer ruling 2026-08-23). + * + * The throwing error's message is carried as free text in `actual`. + */ + | 'unreachable'; /** * A single divergence entry. Produced by the validation gate (ADR §5.2)