Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .changeset/schema-diff-unreachable-kind.md
Original file line numberDiff line numberDiff line change
@@ -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<SchemaDiffEntryKind, …>`) 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.
88 changes: 88 additions & 0 deletions packages/runtime/src/external-validation-plugin.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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({});
Expand DownExpand Up@@ -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)', () => {
Expand DownExpand Up@@ -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': {
Expand Down
66 changes: 60 additions & 6 deletions packages/runtime/src/external-validation-plugin.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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).
*/
Expand DownExpand Up@@ -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);
}
}

Expand DownExpand Up@@ -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;
}
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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' }),
]);
});
});

/**
Expand DownExpand Up@@ -577,14 +649,16 @@ 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);
expect(report.results[0]).toMatchObject({
ok: false,
datasource: 'wh_b',
object: 'wh_b_orders',
diffs: [expect.objectContaining({ kind: 'missing_table', severity: 'error' })],
diffs: [expect.objectContaining({ kind: 'unreachable', severity: 'error' })],
});
});

Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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<SchemaValidationReport> {
const results = await Promise.all(
Expand 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',
},
],
Expand Down
Loading
Loading