From ecf2ac023020e5159b88de3fe19de38f790b944b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 06:57:29 +0000 Subject: [PATCH 1/2] fix(metadata): stop judging Postgres' write-path missing-COLUMN phrase as a missing TABLE (#6347) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `MISSING_TABLE`'s message test demands table/relation next to `does not exist`, but Postgres' write-path phrasing — `column "label" of relation "sys_team" does not exist` — carries a complete legal missing-table phrase as a SUBSTRING, so it matched. The function's own docblock names 42703 as a failure that must stay loud. A message regex can never exclude a superstring, so the repair is a front-exclusion evaluated before any positive test: the column-level SQLSTATEs the docblock already names (42703 / 42704 / 3D000) and the `"x" of relation "y"` sub-object phrasing. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01KDU3qAuJyajAQm3GkUXdfA --- .../src/utils/schema-sync-errors.test.ts | 185 ++++++++++++++++++ .../metadata/src/utils/schema-sync-errors.ts | 98 +++++++++- 2 files changed, 280 insertions(+), 3 deletions(-) diff --git a/packages/metadata/src/utils/schema-sync-errors.test.ts b/packages/metadata/src/utils/schema-sync-errors.test.ts index fd187090da..e73b659d7a 100644 --- a/packages/metadata/src/utils/schema-sync-errors.test.ts +++ b/packages/metadata/src/utils/schema-sync-errors.test.ts @@ -268,3 +268,188 @@ describe('the two classifications are independent, not complementary', () => { expect(isMissingTableError(err)).toBe(false); }); }); + +/** + * #6347 — the phrase corpus. + * + * The defect this pins is a **substring** one, so it cannot be caught by + * checking one phrase at a time: Postgres' write-path missing-COLUMN message, + * `column "label" of relation "sys_team" does not exist`, contains a complete + * legal missing-TABLE phrase (`relation "sys_team" does not exist`) inside it. + * Measured on `origin/main` before the fix, straight against the signature's + * message regex: write-path phrase => `true`, read-path phrase + * (`column "bogus" does not exist`) => `false`. Only the second half was ever + * pinned (see "rejects other 'does not exist' objects" above), which is why the + * first half survived — the test suite asked about the phrasing the consumer's + * own read path produces, and the hole is in the other one. + * + * So the corpus is driven as a table, both directions in one place: + * + * - `LOUD` — must NEVER be judged benign. A false benign here is not a wrong + * log line: `DatabaseLoader.nextEventSeq` answers `return 1` on `true`, so a + * misjudged column error restarts `event_seq` at 1 against a history table + * full of rows, and the colliding insert SUCCEEDS. + * - `BENIGN` — must STILL be judged benign, verbatim. An exclusion is a + * subtraction, and the guarded surface may not shrink while it is applied. + * + * Reverse verification, direction predicted BEFORE running (2026-08-08): + * deleting `MISSING_TABLE.excludes` turns the write-path rows of `LOUD` — and + * only those — RED, while every `BENIGN` row and every read-path row stays + * green. Measured after writing them: 6 failed, exactly the 6 write-path/ + * sub-object-phrase rows, no others. The read-path rows are green in both + * directions on purpose: they were never the hole, and a corpus that could not + * tell the two apart would be reporting the wrong repair. + */ +describe('isMissingTableError — Postgres sub-object phrases are NOT missing tables (#6347)', () => { + /** Errors that must stay LOUD — `isMissingTableError` must answer `false`. */ + const LOUD: ReadonlyArray = [ + // ── the reported hole: the write-path (INSERT/UPDATE/ALTER) phrasing ── + [ + 'PG write-path missing column, bare message', + new Error('column "label" of relation "sys_team" does not exist'), + ], + [ + 'PG write-path missing column, with SQLSTATE 42703', + Object.assign(new Error('column "label" of relation "sys_team" does not exist'), { + code: '42703', + }), + ], + [ + 'PG write-path missing column, thrown as a bare string', + 'column "label" of relation "sys_team" does not exist', + ], + [ + 'PG write-path missing column, wrapped as `cause`', + Object.assign(new Error('insert into "sys_team" failed'), { + cause: Object.assign( + new Error('column "label" of relation "sys_team" does not exist'), + { code: '42703' }, + ), + }), + ], + [ + 'PG write-path missing column, single-quoted identifiers', + new Error("column 'label' of relation 'sys_team' does not exist"), + ], + // ── the same shape for other sub-objects of an EXISTING relation ── + [ + 'PG missing constraint of a relation (42704)', + Object.assign( + new Error('constraint "uq_sys_team_name" of relation "sys_team" does not exist'), + { code: '42704' }, + ), + ], + // ── the read-path phrasing: already excluded before #6347, kept pinned ── + [ + 'PG read-path missing column, bare message', + new Error('column "bogus" does not exist'), + ], + [ + 'PG read-path missing column, with SQLSTATE 42703', + Object.assign(new Error('column "bogus" does not exist'), { code: '42703' }), + ], + // ── code-first: 42703 means not-a-table whatever the prose says ── + [ + '42703 with an opaque message', + Object.assign(new Error('db error'), { code: '42703' }), + ], + [ + '42703 whose message is a genuine missing-table phrase (code wins, loudly)', + Object.assign(new Error('relation "sys_team" does not exist'), { code: '42703' }), + ], + // ── the docblock's other named neighbours, still excluded ── + [ + 'PG missing role (42704)', + Object.assign(new Error('role "app_rw" does not exist'), { code: '42704' }), + ], + [ + 'PG missing database (3D000)', + Object.assign(new Error('database "objectstack" does not exist'), { code: '3D000' }), + ], + // ── other dialects' missing-column prose, for completeness ── + [ + 'MySQL unknown column (ER_BAD_FIELD_ERROR)', + Object.assign(new Error("Unknown column 'label' in 'field list'"), { + code: 'ER_BAD_FIELD_ERROR', + errno: 1054, + }), + ], + [ + 'SQLite unknown column on a read', + Object.assign(new Error('no such column: bogus'), { code: 'SQLITE_ERROR' }), + ], + [ + 'SQLite unknown column on a write', + Object.assign(new Error('table sys_team has no column named label'), { + code: 'SQLITE_ERROR', + }), + ], + ]; + + /** Errors that must STILL be benign — the guarded surface, verbatim. */ + const BENIGN: ReadonlyArray = [ + ['SQLite / libsql message', new Error('no such table: sys_metadata_history')], + ['SQLite message with a schema prefix', new Error('no such table: main.sys_metadata_history')], + [ + 'libsql remote, driver prefix in front of the phrase', + new Error('SQLITE_UNKNOWN: SQLite error: no such table: sys_metadata_history'), + ], + ['PG message', new Error('relation "sys_metadata_history" does not exist')], + [ + 'PG SQLSTATE 42P01 with an opaque message', + Object.assign(new Error('db error'), { code: '42P01' }), + ], + ['MySQL message', new Error("Table 'app.sys_metadata_history' doesn't exist")], + [ + 'MySQL ER_NO_SUCH_TABLE', + Object.assign(new Error('opaque'), { code: 'ER_NO_SUCH_TABLE' }), + ], + ['MySQL errno 1146', Object.assign(new Error('opaque'), { errno: 1146 })], + ['a bare thrown string', 'no such table: sys_metadata_history'], + [ + 'genuine missing table wrapped as `cause`', + Object.assign(new Error('find failed'), { + cause: Object.assign(new Error('relation "sys_metadata_history" does not exist'), { + code: '42P01', + }), + }), + ], + ]; + + it.each(LOUD)('stays loud: %s', (_name, error) => { + expect(isMissingTableError(error)).toBe(false); + }); + + it.each(BENIGN)('still benign: %s', (_name, error) => { + expect(isMissingTableError(error)).toBe(true); + }); + + it('does not let a sub-object phrase be rescued by a missing-table `cause`', () => { + // Recognising "a column of an existing relation" ENDS the question: the + // exclusion returns false without descending. The alternative — keep + // walking and let a nested 42P01 win — would put the corrupting verdict + // back one level down, reachable by any driver that wraps. + const err = Object.assign( + new Error('column "label" of relation "sys_team" does not exist'), + { + code: '42703', + cause: Object.assign(new Error('relation "sys_team" does not exist'), { + code: '42P01', + }), + }, + ); + expect(isMissingTableError(err)).toBe(false); + }); + + it('leaves the DDL predicate alone — `already exists` on a column still matches', () => { + // The exclusion is scoped to MISSING_TABLE. `ALREADY_EXISTS` documents + // that Postgres' own `column "x" of relation "y" already exists` matches + // its test, and it is genuinely benign there: the column IS provisioned. + const err = Object.assign( + new Error('column "environment_id" of relation "sys_metadata" already exists'), + { code: '42701' }, + ); + expect(isSchemaAlreadyExistsError(err)).toBe(true); + expect(isMissingTableError(err)).toBe(false); + }); +}); diff --git a/packages/metadata/src/utils/schema-sync-errors.ts b/packages/metadata/src/utils/schema-sync-errors.ts index f23bd8cb6e..8a11cf4ee0 100644 --- a/packages/metadata/src/utils/schema-sync-errors.ts +++ b/packages/metadata/src/utils/schema-sync-errors.ts @@ -91,6 +91,21 @@ interface DriverErrorSignature { readonly errnos: ReadonlySet; /** `error.message` — the only signal SQLite-family drivers give. */ readonly message: RegExp; + /** + * Optional **front-exclusion**, evaluated before any positive test (#6347). + * + * A message test can never exclude a *superstring*: once a legal phrase for + * X appears inside a longer phrase that means NOT-X, no amount of widening + * the X regex removes the match — the phrase really is in there. The only + * repair is to recognise the not-X shape first and stop. So this is a + * separate channel rather than another alternation in {@link message}. + */ + readonly excludes?: { + /** SQLSTATEs / driver codes that positively mean "**not** this case". */ + readonly codes: ReadonlySet; + /** Message shapes that carry a legal match for this case as a substring. */ + readonly message: RegExp; + }; } /** @@ -132,6 +147,27 @@ const ALREADY_EXISTS: DriverErrorSignature = { * 1" would be the wrong answer against a table that may be full of rows. So the * message test demands the word table/relation next to the phrase rather than * the phrase alone, and the code set carries only the table-scoped SQLSTATEs. + * + * That was not enough on its own, and #6347 is why. Postgres has **two** + * missing-column phrasings, one per direction: + * + * | path | phrase | SQLSTATE | matched the message test? | + * |:---|:---|:---|:---| + * | read (`SELECT`) | `column "bogus" does not exist` | 42703 | no | + * | write (`INSERT`/`UPDATE`/`ALTER`) | `column "label" of relation "sys_team" does not exist` | 42703 | **yes** | + * + * The write-path phrase contains a complete, legal missing-table phrase — + * `relation "sys_team" does not exist` — as a substring, so the table-scoped + * test above matched it and answered *benign* about an error the docblock two + * paragraphs up already named as one that must stay loud. The same holds for + * every other sub-object of a relation Postgres phrases this way, e.g. + * `constraint "uq_x" of relation "sys_team" does not exist` (42704). And + * code-first does not rescue it: {@link matchesDriverError} is a sequential OR, + * so a `code: '42703'` error simply falls past the two code lines and is + * decided by the message. + * + * Hence {@link DriverErrorSignature.excludes}: the not-a-table shapes are + * recognised FIRST, and recognition ends the question with `false`. */ const MISSING_TABLE: DriverErrorSignature = { codes: new Set([ @@ -146,18 +182,61 @@ const MISSING_TABLE: DriverErrorSignature = { */ message: /no such table|relation ["'`][^"'`]+["'`] does not exist|table ["'`][^"'`]+["'`] doesn'?t exist|unknown table/i, + excludes: { + /** + * Exactly the three SQLSTATEs the docblock above already names as + * must-stay-loud neighbours of `does not exist`. They are listed here + * rather than merely trusted to miss the message test, because two of + * them (42703 columns, 42704 constraints/triggers) have a phrasing that + * *does* hit it, and because a code is a fact where prose is a guess. + * + * Postgres-shaped on purpose: measured, neither MySQL + * (`Unknown column 'label' in 'field list'`) nor SQLite + * (`no such column: bogus`, `table t has no column named label`) + * phrases a sub-object failure so that a missing-table phrase falls out + * of it, so there is nothing there to exclude. Adding their codes would + * be surface with no defect behind it. + */ + codes: new Set([ + '42703', // undefined_column + '42704', // undefined_object — constraint, trigger, role, type, … + '3D000', // invalid_catalog_name — `database "x" does not exist` + ]), + /** + * `«sub-object» "x" of relation "y" …` — Postgres' phrasing for a + * failure about something *inside* a relation, which therefore says the + * relation itself is present. The two in-repo siblings that already + * carry this phrase are `mapDataError` (`packages/rest`, #5352) and + * `MISSING_COLUMN_OF_RELATION` (`service-analytics`, #6035/PR #6346); + * this is a deliberate one-line copy rather than a cross-package import, + * and deliberately **wider** than theirs. Both of those *extract* the + * column name to phrase a better error, so a miss costs a vaguer + * message; this one *excludes*, so a miss restores the corruption. It + * therefore drops their `column`/`[a-z0-9_]+`/`does not exist` anchors: + * any sub-object, any quoted identifier, any verdict. Over-matching here + * only ever converts a benign verdict into a loud one, which is the + * direction this whole module already errs in. + */ + message: /["'`][^"'`]+["'`]\s+of relation\s/i, + }, }; /** How far to follow an `error.cause` chain — drivers wrap, but not deeply. */ const MAX_CAUSE_DEPTH = 4; /** - * The single matcher both predicates run on: code, then errno, then message, - * then one step down the `cause` chain. + * The single matcher both predicates run on: exclusions, then code, then errno, + * then message, then one step down the `cause` chain. * * Unrecognised is always `false` — a benign verdict must be *earned*, never * defaulted to, because a false "benign" corrupts data while a false "real" * costs one error line. + * + * The exclusion runs at every node and, when it fires, returns `false` **without + * descending into `cause`** (#6347). Two reasons, both the conservative + * direction: an error that positively identifies as "a column of an existing + * relation" *is* that error, whatever it wraps; and stopping can only ever + * subtract benign verdicts, never add one. */ function matchesDriverError( error: unknown, @@ -166,7 +245,10 @@ function matchesDriverError( ): boolean { if (error === null || error === undefined || depth > MAX_CAUSE_DEPTH) return false; - if (typeof error === 'string') return signature.message.test(error); + if (typeof error === 'string') { + if (signature.excludes?.message.test(error)) return false; + return signature.message.test(error); + } if (typeof error !== 'object') return false; const err = error as { @@ -176,6 +258,12 @@ function matchesDriverError( cause?: unknown; }; + const excludes = signature.excludes; + if (excludes) { + if (typeof err.code === 'string' && excludes.codes.has(err.code)) return false; + if (typeof err.message === 'string' && excludes.message.test(err.message)) return false; + } + if (typeof err.code === 'string' && signature.codes.has(err.code)) return true; if (typeof err.errno === 'number' && signature.errnos.has(err.errno)) return true; if (typeof err.message === 'string' && signature.message.test(err.message)) return true; @@ -208,6 +296,10 @@ export function isSchemaAlreadyExistsError(error: unknown, depth = 0): boolean { * caller must report the consequence and give up rather than compute an answer * from data it never read (#4825). * + * A failure about a **column** of a relation is never this case, in either of + * Postgres' two phrasings — the relation is right there in the message because + * it exists (#6347). See {@link MISSING_TABLE}'s `excludes`. + * * @param error - The value thrown by a driver/engine read (`find`, `findOne`, …). * @param depth - Internal `cause`-chain recursion counter; callers pass nothing. * @returns `true` only when the error positively identifies as From 8d39a27f09b846d6e871b91119f15f0512af1e05 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 07:09:22 +0000 Subject: [PATCH 2/2] docs(metadata): record the measured reverse-verification result + changeset (#6347) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01KDU3qAuJyajAQm3GkUXdfA --- .../missing-table-column-of-relation.md | 44 +++++++++++++++++++ .../src/utils/schema-sync-errors.test.ts | 16 ++++--- 2 files changed, 54 insertions(+), 6 deletions(-) create mode 100644 .changeset/missing-table-column-of-relation.md diff --git a/.changeset/missing-table-column-of-relation.md b/.changeset/missing-table-column-of-relation.md new file mode 100644 index 0000000000..ca32fcb960 --- /dev/null +++ b/.changeset/missing-table-column-of-relation.md @@ -0,0 +1,44 @@ +--- +"@objectstack/metadata": patch +--- + +fix(metadata): `isMissingTableError` no longer reads Postgres' write-path missing-COLUMN message as a missing TABLE (#6347) + +`isMissingTableError` is the single predicate that licenses a caller to treat a +failed read as "the table is not provisioned yet, so there are genuinely no +rows". Its own docblock names `column "x" does not exist` (SQLSTATE 42703) as a +real failure that must stay loud — "a case where 'start numbering at 1' would be +the wrong answer against a table that may be full of rows" — and the code did +not honour that, in one direction only. + +Postgres has **two** missing-column phrasings: + +| path | message | judged | +|:---|:---|:---| +| read (`SELECT`) | `column "bogus" does not exist` | correctly NOT a missing table | +| write (`INSERT`/`UPDATE`/`ALTER`) | `column "label" of relation "sys_team" does not exist` | **wrongly** a missing table | + +The write-path phrase contains a complete, legal missing-table phrase — +`relation "sys_team" does not exist` — as a substring, so the table-scoped +message test matched it. The code channel did not rescue it either: the matcher +is a sequential OR, so an error carrying `code: '42703'` falls past both code +lines and is decided by its message. The same superstring covers every other +sub-object of a relation Postgres phrases this way, e.g. +`constraint "uq_x" of relation "sys_team" does not exist` (42704). + +A message regex can never exclude a superstring, so the repair is a +**front-exclusion** evaluated before any positive test: the column-level +SQLSTATEs the docblock already names (`42703`, `42704`, `3D000`) and the +`"x" of relation "y"` sub-object phrasing. Recognising one ends the question +with `false` — it does not descend into `cause`, because an error that +identifies as "a column of an existing relation" is that error whatever it +wraps. + +What changes for you: a driver error of that shape now propagates instead of +being silenced. Every consumer of the predicate is affected the same way, and +all of them get louder rather than quieter — `DatabaseLoader.nextEventSeq` and +`SysMetadataRepository`'s history counters no longer restart `event_seq` at 1, +`ObjectQLEngine`'s autonumber seed no longer reseeds from 0, and the metadata +loaders no longer answer "nothing declared". The set of errors judged benign +shrinks; nothing that was loud becomes quiet. Genuine missing-table detection is +unchanged for PostgreSQL, MySQL/MariaDB and the SQLite family. diff --git a/packages/metadata/src/utils/schema-sync-errors.test.ts b/packages/metadata/src/utils/schema-sync-errors.test.ts index e73b659d7a..24e63be946 100644 --- a/packages/metadata/src/utils/schema-sync-errors.test.ts +++ b/packages/metadata/src/utils/schema-sync-errors.test.ts @@ -293,12 +293,16 @@ describe('the two classifications are independent, not complementary', () => { * subtraction, and the guarded surface may not shrink while it is applied. * * Reverse verification, direction predicted BEFORE running (2026-08-08): - * deleting `MISSING_TABLE.excludes` turns the write-path rows of `LOUD` — and - * only those — RED, while every `BENIGN` row and every read-path row stays - * green. Measured after writing them: 6 failed, exactly the 6 write-path/ - * sub-object-phrase rows, no others. The read-path rows are green in both - * directions on purpose: they were never the hole, and a corpus that could not - * tell the two apart would be reporting the wrong repair. + * deleting `MISSING_TABLE.excludes` turns the write-path / sub-object rows of + * `LOUD` RED, while every `BENIGN` row and every read-path row stays green. + * Measured: `8 failed | 48 passed` — the direction held, and the count came in + * two ABOVE the six rows predicted by counting phrases, because two rows go red + * through the code channel rather than the phrase: `42703 whose message is a + * genuine missing-table phrase` and the `cause`-rescue case below. Both are + * part of the same repair (a code is a fact, prose is a guess) and are recorded + * here rather than trimmed to fit the prediction. The read-path rows are green + * in both directions on purpose: they were never the hole, and a corpus that + * could not tell the two apart would be reporting the wrong repair. */ describe('isMissingTableError — Postgres sub-object phrases are NOT missing tables (#6347)', () => { /** Errors that must stay LOUD — `isMissingTableError` must answer `false`. */