From 7658453405bb6af9b303ce42f0f2bdb6885c273e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 02:43:11 +0000 Subject: [PATCH 1/4] wip(metadata-protocol): one shared non-raising table-presence probe Claude-Session: https://claude.ai/code/session_01RuoNSXUbBoWHkNS4AknTrM Co-authored-by: Claude --- .../src/migrations/driver-exec.ts | 36 ++ .../src/migrations/partial-index-probe.ts | 28 +- .../src/migrations/read-probe.ts | 350 ++++++++++++++++++ .../src/migrations/seed-tenancy-backfill.ts | 177 ++++----- .../migrations/sys-setting-identity-index.ts | 82 +++- packages/metadata-protocol/src/plugin.ts | 13 +- 6 files changed, 587 insertions(+), 99 deletions(-) create mode 100644 packages/metadata-protocol/src/migrations/read-probe.ts diff --git a/packages/metadata-protocol/src/migrations/driver-exec.ts b/packages/metadata-protocol/src/migrations/driver-exec.ts index ca09238f31..abe868c4cd 100644 --- a/packages/metadata-protocol/src/migrations/driver-exec.ts +++ b/packages/metadata-protocol/src/migrations/driver-exec.ts @@ -151,3 +151,39 @@ export function resolveDriverExec(driver: IDataDriver | null | undefined): Drive } return undefined; } + +/** + * [#17175] The knex client name `driver` speaks, or `undefined`. + * + * Re-homed here from `seed-tenancy-backfill.ts`'s private `resolveClientName` + * when a SECOND site needed it: `sys-setting-identity-index.ts` has to compile a + * dialect-specific catalog statement through `read-probe.ts`, and a second copy + * of this walk is the duplication this module exists to prevent — the same + * argument its header makes for the three copies of the exec resolution. + * + * ⚠️ Three lookups, in this order, because the answer sits in a different place + * depending on how the driver was built: its own `config.client`, then the knex + * instance's (`knex.client.config.client`), then a knex bound to a context + * (`knex.context.client.config.client`). Each read is individually guarded, so a + * getter that throws yields `undefined` rather than escaping into a boot hook. + * + * `undefined` is a REAL answer — "this host did not say" — and its consumers + * must treat it as one. ⛔ It is never defaulted to a dialect: guessing is what + * `read-probe.ts`'s fence is written against. + */ +export function resolveDriverClientName(driver: unknown): string | undefined { + const candidate = driver as any; + const read = (fn: () => unknown): string | undefined => { + try { + const v = fn(); + return typeof v === 'string' && v.length > 0 ? v : undefined; + } catch { + return undefined; + } + }; + return ( + read(() => candidate?.config?.client) ?? + read(() => candidate?.knex?.client?.config?.client) ?? + read(() => candidate?.knex?.context?.client?.config?.client) + ); +} diff --git a/packages/metadata-protocol/src/migrations/partial-index-probe.ts b/packages/metadata-protocol/src/migrations/partial-index-probe.ts index 76b53c7102..61b8717971 100644 --- a/packages/metadata-protocol/src/migrations/partial-index-probe.ts +++ b/packages/metadata-protocol/src/migrations/partial-index-probe.ts @@ -49,7 +49,7 @@ import { isUniqueViolationError, operatorFacingErrorText } from '@objectstack/types'; -import { driverCanRunSql, resolveDriverExec } from './driver-exec.js'; +import { driverCanRunSql, resolveDriverClientName, resolveDriverExec } from './driver-exec.js'; /** * Raw-SQL seam. The surface is resolved by `./driver-exec.ts`: `execute()` @@ -85,6 +85,26 @@ export type IndexExec = (sql: string) => Promise; * `getDriverForObject` is here to prevent. */ export function resolveIndexExecForTable(engine: unknown, table: string): IndexExec | undefined { + return resolveIndexSeamForTable(engine, table)?.exec; +} + +/** + * The raw-SQL seam for ONE table, PLUS the dialect the driver behind it speaks. + * + * [#17175] The pair, for the same structural reason `seed-tenancy-backfill.ts` + * takes a seam rather than a bare exec (#9381): a statement compiled for a + * dialect nobody resolved is a statement compiled for a guess. The presence + * probe in `read-probe.ts` needs the dialect to pick a catalog arm, and + * {@link resolveIndexExecForTable} — the pre-existing entry point, unchanged in + * signature and behaviour — now reads its `exec` off this. + * + * `client` is `undefined` on a host that does not say, which is a real answer + * and ⛔ never defaulted; the probe falls back to the caller's own statement. + */ +export function resolveIndexSeamForTable( + engine: unknown, + table: string, +): { exec: IndexExec; client?: string } | undefined { const engineAny = engine as any; const attempt = (fn: () => unknown): any => { try { @@ -108,7 +128,11 @@ export function resolveIndexExecForTable(engine: unknown, table: string): IndexE } } if (!canRunSql(driver)) return undefined; - return resolveDriverExec(driver); + const exec = resolveDriverExec(driver); + // `canRunSql` above is defined AS this resolution succeeding, so `exec` is + // present here; the guard is for the type, not for a reachable state. + if (!exec) return undefined; + return { exec, client: resolveDriverClientName(driver) }; } /** diff --git a/packages/metadata-protocol/src/migrations/read-probe.ts b/packages/metadata-protocol/src/migrations/read-probe.ts new file mode 100644 index 0000000000..fd66ad50a4 --- /dev/null +++ b/packages/metadata-protocol/src/migrations/read-probe.ts @@ -0,0 +1,350 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * ═══════════════════════════════════════════════════════════════════════════ + * [#17175] The shared read-probe layer for the `kernel:ready` migrations — + * what a result set looks like on the three dialects, and the ONE + * non-raising table-presence probe built on them. + * ═══════════════════════════════════════════════════════════════════════════ + * + * ## The defect this closes + * + * Two migrations on the `kernel:ready` hook ask "does this table exist?" by + * running a statement that CANNOT succeed when the answer is no: + * + * * `seed-tenancy-backfill.ts` — `SELECT "tenant_id" FROM + * "_objectstack_sequences" WHERE 1 = 0`, on every install that has never + * allocated an autonumber; + * * `sys-setting-identity-index.ts` — `SELECT 1 FROM sys_setting WHERE 1 = 0`, + * on every kernel that does not register the OPTIONAL `service-settings`. + * + * Both catch the refusal and read it as "no". Neither is a defect on its own + * terms. But the refusal travels through `SqlDriver.execute()`, whose raw + * terminal writes the statement and the dialect's message to the operator's + * log on the way out — measured on this tree, exactly one line per probe, on + * `console.warn`, i.e. **stderr**, carrying both the `DATABASE_ERROR` token and + * the dialect's `no such table`: + * + * ``` + * [sql-driver] DATABASE_ERROR — the backend refused a raw statement (SQLITE_ERROR). + * … statement: SELECT "tenant_id" FROM "_objectstack_sequences" WHERE 1 = 0; + * dialect: … - no such table: _objectstack_sequences + * ``` + * + * ⭐ The cost is not the line. It is that operators learn this product prints + * errors when nothing is wrong, and then miss the one that matters. A consumer + * told to read the boot log — `objectstack-ai/hotclm`'s `AGENTS.md` says "a boot + * that logs warnings is not a passing boot", naming `no such table` — must + * either ignore an unactionable line every boot or chase a platform-internal + * probe. `sys-setting-identity-index.ts`'s own docblock states the same cost in + * its own words, while printing one of these lines. + * + * ## ⛔ Why the repair is here and NOT in the driver + * + * The driver cannot tell these two apart, and the reason is structural rather + * than an oversight. Quietening a refusal requires CLASSIFYING it, this repo has + * exactly one predicate for that (`isMissingTableError`, `@objectstack/types`), + * and it needs `readObject` — the name of the thing the caller was reading — + * both to avoid the #13324 fail-open and because + * `driver-error-classification.callers.test.ts` fails any in-repo call that + * omits it. The raw path has no such name: `execute()` takes a string, and + * `rawStatementFaultError` declares no targeted table (pinned by + * `sql-driver-16019-raw-statement-fault-envelope.test.ts`). An unclassified + * demotion of the whole raw terminal would quieten real failures too. + * + * ⇒ The caller knows the table. The driver does not. So the probe moves, not + * the log — and it moves ONCE, here, rather than once per probe site. + * + * ## ⛔ The fence: "absent" and "could not look" are different answers + * + * The failure mode this helper is written against is its own: a catalog arm + * that is mis-compiled for some dialect raises, is caught by the same `catch` + * the expected miss uses, and reads as "the table is not there" — turning a + * stored-row data repair into a SILENT no-op on whichever dialect nobody + * exercised. That is strictly worse than a noisy log. + * + * ⇒ {@link readTablePresence} returns FOUR verdicts, not a boolean, and + * `'unreadable'` is ⛔ never folded into `'absent'`. The caller is required to + * treat it as a report, not as an answer. Both directions are pinned in + * `read-probe.test.ts`; a helper that could not tell them apart would be + * refused however clean it read. + * + * ## Dialect coverage + * + * The three catalog arms are compiled from the knex client name, using the SAME + * three spelling sets `SqlDriver` itself emits SQL for. They are re-spelled here + * rather than imported, for the reason this package already re-spells + * `GLOBAL_TENANT`: `metadata-protocol` must not depend on a driver. + * + * A client in NONE of the three sets gets no catalog statement and falls back to + * the caller's own pre-existing `WHERE 1 = 0` probe — which still raises, and so + * still prints, exactly as it did before this change. That path is not a + * regression and it is not silent: its refusal is now CLASSIFIED with + * `isMissingTableError(error, table)`, so an unrecognised dialect that refuses + * for some OTHER reason reports `'unreadable'` where it used to be swallowed as + * absence. ⛔ Guessing a catalog spelling for an unknown dialect is exactly the + * mis-compiled arm the fence is about, so it is not done. + * + * ⚠️ The three arms must agree on WHAT COUNTS as present, or they answer three + * different questions. All three count tables and views: `sqlite_master` is + * filtered to `('table','view')`, `to_regclass` resolves any relation on the + * search path, and `information_schema.tables` lists both. + */ + +import { isMissingTableError, operatorFacingErrorText } from '@objectstack/types'; + +/** + * A raw-SQL read seam, in the shape BOTH migration seams satisfy. + * + * Deliberately one parameter: `SeedTenancyExec` is `(sql, params?)` and + * `IndexExec` is `(sql)`, and a one-parameter target accepts both. Nothing here + * binds a value — the only thing interpolated is a table name, which cannot be a + * bound parameter in any of the three dialects anyway, and which + * {@link isProbeableTableName} restricts to a plain identifier first. + */ +export type ReadProbeExec = (sql: string) => Promise; + +/** + * Is `result` one of the result-set shapes a raw SELECT can come back as? + * + * The same three {@link normalizeRows} flattens, asked as a yes/no: a bare row + * array (better-sqlite3 through knex), `{ rows }` (pg), and the `[rows, fields]` + * tuple (mysql2). An empty result set in any of those spellings is still a + * result set, and still `true` — that is what keeps a healthy install's + * `no-split` intact, and it is the half of #10789 that stopped it being a + * rename. + * + * This cannot lose a split that {@link normalizeRows} would have found: every + * shape it rejects is one already flattened to `[]`, so the only change is + * "reported as unreadable" replacing "reported as zero rows". + * + * ⛔ NOT exported from the package index. It has no consumer outside this + * package, and the CLI's `migrate/duplicates.ts` carries its own copy for its + * own probes (#10677) — unifying the two is a separate decision, exactly as + * `quoteIdent` records for the same pair. + * + * [#17175] Re-homed here from `seed-tenancy-backfill.ts`, unchanged, so the + * shared presence probe can be built on it without importing from one of its own + * callers. `seed-tenancy-backfill.ts` re-exports it, so every existing importer + * and the package index are untouched. + */ +export function isResultSet(result: unknown): boolean { + if (Array.isArray(result)) return true; + if (typeof result === 'object' && result !== null) { + return Array.isArray((result as { rows?: unknown }).rows); + } + return false; +} + +/** + * Flatten the three result shapes the supported dialects return from a raw + * SELECT into one row list. + * + * `better-sqlite3` (through knex) returns a bare row array; `pg` returns + * `{ rows, rowCount, … }`; `mysql2` returns the tuple `[rows, fields]`. A + * migration that read only one of them would silently see zero rows on the other + * two — and "zero rows" is this module's every-branch no-op, so the failure + * would look exactly like a healthy install. + * + * [#17175] Re-homed here from `seed-tenancy-backfill.ts`, unchanged and still + * re-exported from there, which is what the package index publishes. + */ +export function normalizeRows(result: unknown): Record[] { + if (!result) return []; + if (Array.isArray(result)) { + // mysql2's `[rows, fields]`: the first element is itself the row array. + if (result.length > 0 && Array.isArray(result[0])) { + return result[0] as Record[]; + } + return result as Record[]; + } + const rows = (result as { rows?: unknown }).rows; + return Array.isArray(rows) ? (rows as Record[]) : []; +} + +/** Why a probe was treated as unreadable — the `detail` an operator reads. */ +export const SEAM_NO_ANSWER_DETAIL = + 'the raw-SQL seam returned no result set — a seam that cannot answer is not a seam that answered "no rows"'; + +/** + * SQLite knex client spellings — the set `SqlDriver.SQLITE_EMIT_CLIENTS` holds. + * @see the module header on why these are re-spelled rather than imported. + */ +const SQLITE_CLIENTS: ReadonlySet = new Set(['sqlite3', 'sqlite', 'better-sqlite3']); +/** Postgres knex client spellings — `SqlDriver.POSTGRES_EMIT_CLIENTS`. */ +const POSTGRES_CLIENTS: ReadonlySet = new Set(['postgres', 'pg', 'postgresql', 'pgnative']); +/** MySQL knex client spellings — `SqlDriver.MYSQL_EMIT_CLIENTS`. */ +const MYSQL_CLIENTS: ReadonlySet = new Set(['mysql', 'mysql2']); + +/** + * Which catalog family a knex client name belongs to, or `undefined`. + * + * `undefined` is a REAL answer — "this helper has no catalog statement it can + * honestly compile for that client" — and the caller's fallback probe is what + * answers instead. ⛔ It is never resolved to a default family: a default is a + * guess, and a guessed catalog arm is the fence's failure mode. + */ +function catalogFamilyOf(client?: string): 'sqlite' | 'postgres' | 'mysql' | undefined { + const c = String(client ?? '').toLowerCase(); + if (SQLITE_CLIENTS.has(c)) return 'sqlite'; + if (POSTGRES_CLIENTS.has(c)) return 'postgres'; + if (MYSQL_CLIENTS.has(c)) return 'mysql'; + return undefined; +} + +/** + * Is this a name that may be interpolated into a catalog statement? + * + * The table name reaches the catalog arms as a string LITERAL, not as an + * identifier and not as a bound parameter (`IndexExec` binds nothing). Every + * caller passes a module constant, and the platform's own naming rule is + * `snake_case` machine names, so a name this rejects is one the platform could + * not have written. Rejecting it yields no catalog statement — the same road an + * unrecognised dialect takes — rather than an escaped-literal path nobody tests. + */ +function isProbeableTableName(table: string): boolean { + return /^[A-Za-z_][A-Za-z0-9_]*$/.test(table); +} + +/** + * The catalog statement that answers "is this table here?" WITHOUT raising when + * the answer is no, or `undefined` when none can be compiled. + * + * Each arm returns one row when the relation exists and ZERO rows when it does + * not — never an error, which is the whole point. Exported for the pins: the + * text is what the live PG and MySQL arms are asserted on. + */ +export function buildTablePresenceSql(table: string, client?: string): string | undefined { + if (!isProbeableTableName(table)) return undefined; + switch (catalogFamilyOf(client)) { + case 'sqlite': + // `sqlite_master` is the per-connection catalog; temp tables live in + // `sqlite_temp_master` and are deliberately not counted — nothing this + // package provisions is temporary. + return `SELECT 1 FROM sqlite_master WHERE type IN ('table', 'view') AND name = '${table}'`; + case 'postgres': + // `to_regclass` resolves through `search_path`, which is exactly how the + // driver's own unqualified statements resolve, and answers NULL rather + // than raising for a name that is not there. The quoted argument makes the + // match exact instead of case-folded. + return `SELECT 1 WHERE to_regclass('"${table}"') IS NOT NULL`; + case 'mysql': + // `DATABASE()` scopes to the connected schema — `information_schema.tables` + // without it can see a same-named table in another schema on the server. + return ( + `SELECT 1 FROM information_schema.tables ` + + `WHERE table_schema = DATABASE() AND table_name = '${table}'` + ); + default: + return undefined; + } +} + +/** + * What a presence probe found. FOUR verdicts, ⛔ not a boolean. + * + * `'absent'` is an ANSWER — the catalog was read and the table is not in it. + * `'unreadable'` is a REPORT — the probe could not run, so nothing is known. The + * distinction is the fence in this module's header: folding the second into the + * first is how a mis-compiled arm turns a data repair into a silent no-op. + */ +export type TablePresenceVerdict = + /** The catalog named the relation (or the fallback probe ran and did not refuse). */ + | 'present' + /** The probe ANSWERED, and the relation is not there. The expected miss. */ + | 'absent' + /** + * The seam accepted the statement and returned no result set at all — a + * memory engine's no-op `execute` (#10789). Not a failure and not an answer; + * each caller maps it the way its own history already ruled. + */ + | 'no-answer' + /** ⛔ The probe itself failed, for a reason that is not "no such table". */ + | 'unreadable'; + +export interface TablePresenceResult { + verdict: TablePresenceVerdict; + /** Which arm ran — pinned, so "the catalog arm was taken" is checkable. */ + probe: 'catalog' | 'fallback'; + /** Operator-facing reason. Set for `'no-answer'` and `'unreadable'` only. */ + detail?: string; +} + +export interface TablePresenceProbe { + /** + * The table whose presence is asked. Also the `readObject` the fallback arm's + * classification compares the dialect's phrase against, so a refusal naming + * some OTHER relation is ⛔ not read as this table's absence (#13324). + */ + table: string; + /** The knex client name, when the caller resolved one. */ + client?: string; + /** + * The caller's own pre-#17175 `SELECT … WHERE 1 = 0` statement, used ONLY when + * no catalog arm can be compiled. Taken from the caller rather than built here + * so each site keeps the exact statement its own pins and dialect review + * already cover. + */ + fallbackSql: string; +} + +/** + * Ask whether a table is there, without raising when it is not. + * + * @see the module header for the fence this implements, and for why an + * unrecognised dialect keeps the caller's raising probe instead of getting + * a guessed catalog statement. + */ +export async function readTablePresence( + exec: ReadProbeExec, + probe: TablePresenceProbe, +): Promise { + const catalogSql = buildTablePresenceSql(probe.table, probe.client); + + if (catalogSql !== undefined) { + let result: unknown; + try { + result = await exec(catalogSql); + } catch (error) { + // ⛔ THE FENCE. A catalog statement this helper compiled and the backend + // refused is a defect in this helper, not evidence about the table. It is + // never `'absent'`, and the caller is required to report it. + return { + verdict: 'unreadable', + probe: 'catalog', + detail: + operatorFacingErrorText(error) || + `the table-presence catalog probe for '${probe.table}' was refused`, + }; + } + if (!isResultSet(result)) { + return { verdict: 'no-answer', probe: 'catalog', detail: SEAM_NO_ANSWER_DETAIL }; + } + return { + verdict: normalizeRows(result).length > 0 ? 'present' : 'absent', + probe: 'catalog', + }; + } + + // No catalog arm for this client. The caller's own probe runs, exactly as it + // did before #17175 — it still raises on a missing table, and the driver still + // prints one line — but the refusal is now classified rather than conflated. + try { + const result = await exec(probe.fallbackSql); + if (!isResultSet(result)) { + return { verdict: 'no-answer', probe: 'fallback', detail: SEAM_NO_ANSWER_DETAIL }; + } + return { verdict: 'present', probe: 'fallback' }; + } catch (error) { + if (isMissingTableError(error, probe.table)) { + return { verdict: 'absent', probe: 'fallback' }; + } + return { + verdict: 'unreadable', + probe: 'fallback', + detail: + operatorFacingErrorText(error) || + `the table-presence probe for '${probe.table}' was refused`, + }; + } +} diff --git a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts index a9818b94cd..4465f0bd18 100644 --- a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts +++ b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts @@ -140,7 +140,19 @@ import { operatorFacingErrorText, resolveTenancyPosture } from '@objectstack/typ import { postureEnforcesWall } from '@objectstack/spec/security'; import { DATA_MIGRATION_FLAG_OBJECT, type DataMigrationFlag } from '@objectstack/spec/system'; import type { IndexMigrationLogger } from './partial-index-probe.js'; -import { driverCanRunSql, resolveDriverExec } from './driver-exec.js'; +import { driverCanRunSql, resolveDriverClientName, resolveDriverExec } from './driver-exec.js'; +import { + SEAM_NO_ANSWER_DETAIL, + isResultSet, + normalizeRows, + readTablePresence, +} from './read-probe.js'; + +// [#17175] The result-set primitives moved to './read-probe.js' so the shared +// non-raising presence probe could be built on them without importing from one +// of its own callers. Re-exported unchanged: 'normalizeRows' is what the package +// index publishes, and 'runtime-index-preflight.ts' imports both from here. +export { isResultSet, normalizeRows }; /** The driver-private counter table (`SqlDriver.SEQUENCES_TABLE`). */ export const SEQUENCES_TABLE = '_objectstack_sequences'; @@ -210,7 +222,26 @@ export type SeedTenancyBackfillStatus = /** A split exists but the install holds SEVERAL organizations — no derivable owner. */ | 'skipped-ambiguous-organization' /** The backfill ran. */ - | 'applied'; + | 'applied' + /** + * [#17175] ⛔ The counter table's presence could not be READ — distinct from + * {@link 'absent'}, which is an ANSWER. + * + * Reached only when the presence probe itself was refused for a reason that is + * not "no such table": a catalog statement `read-probe.ts` compiled wrong for + * this dialect, a permission denial, a dropped connection. Folding it into + * `'absent'` — which is where every such refusal used to land — is how a + * stored-row repair turns into a SILENT no-op on a dialect nobody exercised, + * and it is the one way the non-raising probe can go wrong. `detail` carries + * the backend's own text, and the run is reported at `warn` as well as + * returned: nothing was lost, but nothing was looked at either. + * + * ⛔ The name is `runtime-index-preflight.ts`'s, deliberately: that module + * already draws this exact line ("'found nothing' and 'never looked' must not + * read the same") and two spellings of one distinction is how the distinction + * stops being read. + */ + | 'unreadable'; /** One object/field whose counter is split across two partitions. */ export interface SeedTenancySplit { @@ -363,21 +394,12 @@ export function resolveSeedTenancySeam(engine: unknown): SeedTenancySeam | undef * that keeps its config elsewhere. Anything unreadable is `undefined`, which * means "quote the ANSI way" — today's behaviour, unchanged. */ -function resolveClientName(driver: any): string | undefined { - const read = (fn: () => unknown): string | undefined => { - try { - const v = fn(); - return typeof v === 'string' && v.length > 0 ? v : undefined; - } catch { - return undefined; - } - }; - return ( - read(() => driver?.config?.client) ?? - read(() => driver?.knex?.client?.config?.client) ?? - read(() => driver?.knex?.context?.client?.config?.client) - ); -} +/** + * [#17175] Moved to `./driver-exec.js` as `resolveDriverClientName` when + * `sys-setting-identity-index.ts` needed the same walk to compile its own + * catalog statement. Same three lookups, same order, same guards. + */ +const resolveClientName = resolveDriverClientName; /** * The exec half alone, for callers that resolve the dialect themselves. @@ -391,30 +413,6 @@ export function resolveSeedTenancyExec(engine: unknown): SeedTenancyExec | undef return resolveSeedTenancySeam(engine)?.exec; } -/** - * Flatten the three result shapes the supported dialects return from a raw - * SELECT into one row list. - * - * `better-sqlite3` (through knex) returns a bare row array; `pg` returns - * `{ rows, rowCount, … }`; `mysql2` returns the tuple `[rows, fields]`. A - * migration that read only one of them would silently see zero rows on the other - * two — and "zero rows" is this module's every-branch no-op, so the failure - * would look exactly like a healthy install. - */ -export function normalizeRows(result: unknown): Record[] { - if (!result) return []; - if (Array.isArray(result)) { - // mysql2's `[rows, fields]`: the first element is itself the row array. - if (result.length > 0 && Array.isArray(result[0])) { - return result[0] as Record[]; - } - return result as Record[]; - } - const rows = (result as { rows?: unknown }).rows; - if (Array.isArray(rows)) return rows as Record[]; - return []; -} - /** * ── The seam that ACCEPTS a query but never ANSWERS one (#10789) ─────────── * @@ -442,35 +440,13 @@ export function normalizeRows(result: unknown): Record[] { */ /** - * Is `result` one of the result-set shapes a raw SELECT can come back as? - * - * The same three {@link normalizeRows} flattens, asked as a yes/no: a bare row - * array (better-sqlite3 through knex), `{ rows }` (pg), and the `[rows, fields]` - * tuple (mysql2). An empty result set in any of those spellings is still a - * result set, and still `true` — that is what keeps a healthy install's - * `no-split` intact, and it is the half of this change that stops it being a - * rename. - * - * This cannot lose a split that {@link normalizeRows} would have found: every - * shape it rejects is one already flattened to `[]`, so the only change is - * "reported as unreadable" replacing "reported as zero rows". - * - * ⛔ NOT exported from the package index. It has no consumer outside this - * module, and the CLI's `migrate/duplicates.ts` carries its own copy for its own - * probes (#10677) — unifying the two is a separate decision, exactly as - * `quoteIdent` records for the same pair. + * [#17175] {@link isResultSet}, {@link normalizeRows} and + * `SEAM_NO_ANSWER_DETAIL` now live in `./read-probe.js` and are imported at the + * top of this file — the first two re-exported from here so the package index + * and `runtime-index-preflight.ts` see no change. They moved because the shared + * non-raising presence probe is built on them and must not import from one of + * its own callers; their bodies and their doc are unchanged. */ -export function isResultSet(result: unknown): boolean { - if (Array.isArray(result)) return true; - if (typeof result === 'object' && result !== null) { - return Array.isArray((result as { rows?: unknown }).rows); - } - return false; -} - -/** Why a probe was treated as unreadable — the `detail` an operator reads. */ -const SEAM_NO_ANSWER_DETAIL = - 'the raw-SQL seam returned no result set — a seam that cannot answer is not a seam that answered "no rows"'; /** * Run one READ probe and flatten it, failing when the seam answered nothing. @@ -529,7 +505,20 @@ function quoteIdent(name: string, client?: string): string { return `"${name.replace(/"/g, '""')}"`; } -/** Probe statement: does the counter table exist at all? */ +/** + * Probe statement: does the counter table exist at all? + * + * [#17175] ⚠️ No longer what {@link backfillSeedTenancy} runs first. The boot + * path now asks the CATALOG through `read-probe.ts`, because this statement + * cannot answer "no" without being REFUSED, and the driver writes every refusal + * of a raw statement to the operator's log — one `DATABASE_ERROR … no such + * table` line per boot on every install that has never allocated an autonumber. + * + * It is still live, and deliberately unchanged, in two places: it is the + * `fallbackSql` the shared probe runs when the connected dialect has no catalog + * arm, and it is what the live-MySQL pin and `packages/runtime`'s integration + * test issue to reach the same table. ⛔ Do not re-point the boot path at it. + */ export function buildSequencesPresenceSql(client?: string): string { return `SELECT ${quoteIdent('tenant_id', client)} FROM ${quoteIdent(SEQUENCES_TABLE, client)} WHERE 1 = 0`; } @@ -1223,20 +1212,42 @@ export async function backfillSeedTenancy( // 1. Is there a counter table at all? Absent on a memory engine, and on any // install that has never allocated an autonumber. // - // TWO ways this probe fails to find one, and the second is #10789. The - // table can be missing — the driver raises, and the `catch` reports it. Or - // the SEAM can be one that accepts the statement and never runs it: a no-op - // `execute` that returns `null` neither throws nor is absent, so this - // branch was unreachable on a memory engine despite the comment above - // saying it was the case it existed for. Both mean "no counter table was - // read", which is what `absent` says; `detail` separates the reasons. - try { - if (!isResultSet(await exec(buildSequencesPresenceSql(client)))) { - return { status: 'absent', ...empty, detail: SEAM_NO_ANSWER_DETAIL }; - } - } catch { - return { status: 'absent', ...empty }; + // [#17175] This used to ASK by running `buildSequencesPresenceSql` — a + // statement that cannot succeed when the answer is no — and reading the + // refusal as the answer. It worked, and it made `SqlDriver.execute()` write + // one `[sql-driver] DATABASE_ERROR … no such table` line to stderr on every + // boot of every install with no counter table. Nothing was broken and the + // operator was told something was. The question is now asked of the + // CATALOG, which answers "no" with zero rows instead of a refusal; the + // statement above is kept and is still what runs on a dialect + // `read-probe.ts` has no catalog arm for. + // + // FOUR verdicts, and the third and fourth are the two #10789 named. A seam + // that accepts the statement and never runs it (a memory engine's no-op + // `execute`) answers nothing — still `absent`, still separated by + // `detail`, exactly as ruled. ⛔ But `'unreadable'` is NOT folded in with + // them: a probe that was refused for a reason other than "no such table" + // looked at nothing, and calling that "the table is not there" is how this + // repair would decline in silence on a dialect nobody exercised. + const presence = await readTablePresence(exec, { + table: SEQUENCES_TABLE, + client, + fallbackSql: buildSequencesPresenceSql(client), + }); + if (presence.verdict === 'unreadable') { + logger?.warn?.( + `[metadata-protocol] the seed/API tenancy repair (#8686) could not read whether ` + + `"${SEQUENCES_TABLE}" exists, so it did NOT run and nothing was changed. This is not the ` + + `table being absent — that answer is silent and normal. Verify by hand with: ` + + `${buildSequencesPresenceSql(client)} (#17175).`, + { error: presence.detail, probe: presence.probe }, + ); + return { status: 'unreadable', ...empty, detail: presence.detail }; + } + if (presence.verdict === 'no-answer') { + return { status: 'absent', ...empty, detail: presence.detail ?? SEAM_NO_ANSWER_DETAIL }; } + if (presence.verdict === 'absent') return { status: 'absent', ...empty }; // 2. Which objects are split? Probed FIRST so a healthy install — the // overwhelming majority, including every fresh boot before sign-up — pays diff --git a/packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts b/packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts index d87258f141..895f4aaf43 100644 --- a/packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts +++ b/packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts @@ -152,10 +152,12 @@ import { logProblem, probeThenReplaceIndex, resolveIndexExecForTable, + resolveIndexSeamForTable, type IndexExec, type IndexMigrationLogger, type PartialIndexStatus, } from './partial-index-probe.js'; +import { readTablePresence } from './read-probe.js'; /** The one table this migration touches. */ export const SYS_SETTING_TABLE = 'sys_setting'; @@ -418,21 +420,61 @@ export function resolveSysSettingIndexExec(engine: unknown): IndexExec | undefin return resolveIndexExecForTable(engine, SYS_SETTING_TABLE); } +/** + * [#17175] The same seam PLUS the dialect, which the presence probe needs to + * pick a catalog arm. @see resolveIndexSeamForTable + */ +export function resolveSysSettingIndexSeam( + engine: unknown, +): { exec: IndexExec; client?: string } | undefined { + return resolveIndexSeamForTable(engine, SYS_SETTING_TABLE); +} + /** * Is `sys_setting` present on the other end of this seam? * - * A thrown error is read as "not present". That is wider than absence strictly - * warrants — a permission error lands here too — and it is the right width: on - * any host where the framework cannot even SELECT from the table, it certainly - * cannot rebuild its index, and reporting one unactionable finding per boot is - * how the actionable ones stop being read. + * [#17175] This used to ASK by running {@link buildSysSettingPresenceSql} and + * reading the refusal as "not present" — which is correct, and which made + * `SqlDriver.execute()` write one `[sql-driver] DATABASE_ERROR … no such table: + * sys_setting` line to the operator's log on EVERY boot of every kernel that + * does not register the optional `service-settings`. The docblock above states + * the cost of that in its own words ("reporting one unactionable finding per + * boot is how the actionable ones stop being read") — while paying it. + * + * The question is now asked of the CATALOG, which answers "no" with zero rows + * instead of a refusal. The statement above still runs, unchanged, on a dialect + * `read-probe.ts` has no catalog arm for. + * + * ⛔ What is NOT kept is the old width. "A thrown error is read as not present" + * folded a permission denial and a dropped connection into absence; those now + * answer `'unreadable'`, and this migration REPORTS them instead of no-opping + * in silence. The previous index is kept either way — the difference is whether + * anyone is told the tightening did not even look. + * + * A seam that accepts the statement and answers nothing ({@link 'no-answer'}) + * keeps this seam's own long-standing reading: only a REFUSAL was ever + * information here, so a no-answer still proceeds, exactly as before. */ -async function tableIsPresent(exec: IndexExec): Promise { - try { - await exec(buildSysSettingPresenceSql()); - return true; - } catch { - return false; +async function tableIsPresent( + exec: IndexExec, + client?: string, +): Promise<{ verdict: 'present' | 'absent' | 'unreadable'; detail?: string }> { + const presence = await readTablePresence(exec, { + table: SYS_SETTING_TABLE, + client, + fallbackSql: buildSysSettingPresenceSql(), + }); + switch (presence.verdict) { + case 'absent': + return { verdict: 'absent' }; + // ⛔ Never folded into 'absent' — see the note above and read-probe.ts's + // fence. "Could not look" is a report, not an answer. + case 'unreadable': + return { verdict: 'unreadable', detail: presence.detail }; + // 'present' and 'no-answer' both proceed: on THIS seam only a refusal + // was ever information. + default: + return { verdict: 'present' }; } } @@ -448,9 +490,25 @@ async function tableIsPresent(exec: IndexExec): Promise { export async function ensureSysSettingIdentityIndex( exec: IndexExec | undefined, logger?: EnsureSysSettingIndexLogger, + opts: { client?: string } = {}, ): Promise { if (!exec) return { status: 'no-driver' }; - if (!(await tableIsPresent(exec))) return { status: 'absent' }; + const presence = await tableIsPresent(exec, opts.client); + if (presence.verdict === 'absent') return { status: 'absent' }; + if (presence.verdict === 'unreadable') { + // [#17175] Not 'absent'. The tightening did not run, the previous index + // is kept, and — unlike absence, which is normal and silent — an + // operator is told, because nothing here looked at anything. + logProblem( + logger, + `[metadata-protocol] could not read whether "${SYS_SETTING_TABLE}" exists, so the row-identity ` + + `index tightening (#8629) did NOT run and the table keeps whatever unique index it had. ` + + `⛔ This is not the table being absent, which is a normal, silent no-op on a kernel without ` + + `service-settings. Verify by hand with: ${buildSysSettingPresenceSql()} (#17175).`, + presence.detail ?? '', + ); + return { status: 'failed', detail: presence.detail }; + } // The probe-first order — prove the NULL-safe form is possible under a // throwaway name, and only THEN drop the declared name and rebuild it — diff --git a/packages/metadata-protocol/src/plugin.ts b/packages/metadata-protocol/src/plugin.ts index f9c18f9551..e43ae638a1 100644 --- a/packages/metadata-protocol/src/plugin.ts +++ b/packages/metadata-protocol/src/plugin.ts @@ -37,7 +37,7 @@ import { } from './migrations/view-definition-active-index.js'; import { ensureSysSettingIdentityIndex, - resolveSysSettingIndexExec, + resolveSysSettingIndexSeam, } from './migrations/sys-setting-identity-index.js'; import { backfillSeedTenancy, @@ -318,7 +318,16 @@ export function assembleMetadataProtocol( ); } try { - await ensureSysSettingIdentityIndex(resolveSysSettingIndexExec(ql), ctx.logger); + // [#17175] The SEAM, not the bare exec: the presence + // probe compiles a catalog statement for the connected + // dialect, and a dialect nobody resolved is a dialect + // guessed. Without it the probe falls back to the + // `WHERE 1 = 0` statement whose refusal this card is + // about. + const seam = resolveSysSettingIndexSeam(ql); + await ensureSysSettingIdentityIndex(seam?.exec, ctx.logger, { + client: seam?.client, + }); } catch (e: unknown) { ctx.logger.warn( '[metadata-protocol] sys_setting row-identity index migration skipped (#8629)', From 0c5f4f85981be5ca386206a33374c8f4f7181597 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 03:04:54 +0000 Subject: [PATCH 2/4] wip(metadata-protocol): pins for the shared presence probe Claude-Session: https://claude.ai/code/session_01RuoNSXUbBoWHkNS4AknTrM Co-authored-by: Claude --- .../raw-exec-operator-detail-16657.test.ts | 2 + .../src/migrations/read-probe.test.ts | 265 ++++++++++++++++++ .../src/migrations/read-probe.testkit.ts | 53 ++++ .../seed-tenancy-backfill.live-mysql.test.ts | 62 +++- .../seed-tenancy-backfill.null-seam.test.ts | 41 ++- .../migrations/seed-tenancy-backfill.test.ts | 16 +- ...nancy-autonumber-split.integration.test.ts | 128 +++++++++ 7 files changed, 557 insertions(+), 10 deletions(-) create mode 100644 packages/metadata-protocol/src/migrations/read-probe.test.ts create mode 100644 packages/metadata-protocol/src/migrations/read-probe.testkit.ts diff --git a/packages/metadata-protocol/src/migrations/raw-exec-operator-detail-16657.test.ts b/packages/metadata-protocol/src/migrations/raw-exec-operator-detail-16657.test.ts index 3d7730013e..305049afca 100644 --- a/packages/metadata-protocol/src/migrations/raw-exec-operator-detail-16657.test.ts +++ b/packages/metadata-protocol/src/migrations/raw-exec-operator-detail-16657.test.ts @@ -73,6 +73,7 @@ import { ORGANIZATION_TABLE, SEQUENCES_TABLE, } from './seed-tenancy-backfill.js'; +import { TABLE_IS_PRESENT_ROWS, isTablePresenceCatalogSql } from './read-probe.testkit.js'; /** `rawStatementFaultError`'s composed message, verbatim (`sql-driver.ts`). */ const COMPOSED = @@ -230,6 +231,7 @@ describe('[#16657] seed-tenancy-backfill — the stored operator record', () => function seamExec(refuse: (sql: string) => boolean) { return async (sql: string): Promise => { if (refuse(sql)) throw rawStatementFault(); + if (isTablePresenceCatalogSql(sql, SEQUENCES_TABLE)) return TABLE_IS_PRESENT_ROWS; if (sql.includes('WHERE 1 = 0')) return []; if (sql.includes('LEFT JOIN')) { return [ diff --git a/packages/metadata-protocol/src/migrations/read-probe.test.ts b/packages/metadata-protocol/src/migrations/read-probe.test.ts new file mode 100644 index 0000000000..a9f4112395 --- /dev/null +++ b/packages/metadata-protocol/src/migrations/read-probe.test.ts @@ -0,0 +1,265 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#17175] The shared non-raising table-presence probe. + * + * ## What this file is FOR, in one line + * + * Two things, and the second is the one that can be got wrong quietly: that a + * recognised dialect never runs the raising probe at all, and that "the catalog + * says no" and "the probe could not run" never answer the same. + * + * ## ⛔ The fence, pinned in BOTH directions + * + * The failure mode of this repair is its own: a catalog arm mis-compiled for + * some dialect raises, is caught by the same \`catch\` the expected miss uses, + * and reads as "the table is not there" — turning a stored-row data repair into + * a SILENT no-op on whichever dialect nobody exercised. That is strictly worse + * than the noisy log this card removes. So both directions are asserted here: + * an ANSWERED absence is \`'absent'\`, and a refused probe is \`'unreadable'\` and + * never \`'absent'\`. + * + * ## Dialect coverage, stated rather than implied + * + * The statement TEXT of all three arms is pinned here, and it is pinned against + * every knex client spelling \`SqlDriver\` emits for, so a spelling dropped from + * one family does not silently fall through to the fallback probe. + * + * ⚠️ Running those statements against a live SERVER is a different claim, and + * this file does not make it. The MySQL arm is executed against a real server in + * \`seed-tenancy-backfill.live-mysql.test.ts\`; the SQLite arm end to end against + * a real \`SqlDriver\` in \`packages/runtime\`'s + * \`seed-tenancy-autonumber-split.integration.test.ts\`. ⛔ The POSTGRES arm is + * NOT MEASURED against a live server anywhere — this package has no live-PG + * harness, no \`pg\` dependency, and its CI leg supplies \`OS_TEST_MYSQL_URL\` + * only while filtering to \`live-mysql\`. Recorded here rather than left to be + * discovered. + */ + +import { describe, it, expect } from 'vitest'; +import { buildTablePresenceSql, readTablePresence, type ReadProbeExec } from './read-probe.js'; +import { SEQUENCES_TABLE } from './seed-tenancy-backfill.js'; + +const TABLE = SEQUENCES_TABLE; +/** The caller's pre-#17175 statement — what the fallback arm runs, and nothing else may. */ +const FALLBACK_SQL = `SELECT "tenant_id" FROM "${TABLE}" WHERE 1 = 0`; + +const SQLITE_CLIENTS = ['sqlite3', 'sqlite', 'better-sqlite3']; +const POSTGRES_CLIENTS = ['postgres', 'pg', 'postgresql', 'pgnative']; +const MYSQL_CLIENTS = ['mysql', 'mysql2']; + +/** A seam that records every statement it is handed, then answers `answer(sql)`. */ +function recordingExec(answer: (sql: string) => unknown): { exec: ReadProbeExec; seen: string[] } { + const seen: string[] = []; + return { + seen, + exec: async (sql: string) => { + seen.push(sql); + const out = answer(sql); + if (out instanceof Error) throw out; + return out; + }, + }; +} + +const ONE_ROW = [{ present: 1 }]; + +describe('[#17175] buildTablePresenceSql — one arm per dialect family, and no guessing', () => { + it('every SQLite spelling compiles the sqlite_master arm', () => { + for (const client of SQLITE_CLIENTS) { + expect(buildTablePresenceSql(TABLE, client)).toBe( + `SELECT 1 FROM sqlite_master WHERE type IN ('table', 'view') AND name = '${TABLE}'`, + ); + } + }); + + it('every Postgres spelling compiles the to_regclass arm', () => { + for (const client of POSTGRES_CLIENTS) { + expect(buildTablePresenceSql(TABLE, client)).toBe( + `SELECT 1 WHERE to_regclass('"${TABLE}"') IS NOT NULL`, + ); + } + }); + + it('every MySQL spelling compiles the information_schema arm, scoped to the connected schema', () => { + for (const client of MYSQL_CLIENTS) { + expect(buildTablePresenceSql(TABLE, client)).toBe( + `SELECT 1 FROM information_schema.tables ` + + `WHERE table_schema = DATABASE() AND table_name = '${TABLE}'`, + ); + } + }); + + it('no arm can be refused for the reason the old probe was — none names the table in a FROM', () => { + // The whole mechanism, asserted rather than described: the statement the + // probe runs does not read FROM the table it is asking about, so a missing + // table cannot make it fail. That is what stops the driver composing a + // `DATABASE_ERROR` line on the happy path. + for (const client of [...SQLITE_CLIENTS, ...POSTGRES_CLIENTS, ...MYSQL_CLIENTS]) { + const sql = buildTablePresenceSql(TABLE, client) as string; + expect(sql).not.toContain('WHERE 1 = 0'); + expect(sql).not.toContain(`FROM "${TABLE}"`); + expect(sql).not.toContain(`FROM \`${TABLE}\``); + expect(sql).not.toMatch(new RegExp(`FROM\\s+${TABLE}\\b`)); + } + }); + + it('⛔ an unrecognised client compiles NOTHING — a default arm would be a guess', () => { + for (const client of ['oracledb', 'mssql', 'cockroachdb', 'libsql', '', undefined]) { + expect(buildTablePresenceSql(TABLE, client)).toBeUndefined(); + } + }); + + it('⛔ a name that is not a plain identifier compiles nothing — it reaches SQL as a literal', () => { + for (const table of [`x'; DROP TABLE y; --`, 'has space', 'has"quote', '1leading_digit', '']) { + for (const client of ['better-sqlite3', 'pg', 'mysql2']) { + expect(buildTablePresenceSql(table, client)).toBeUndefined(); + } + } + }); +}); + +describe('[#17175] readTablePresence — the catalog arm, on a recognised dialect', () => { + it('a row means present, and ⛔ the raising probe is never issued', async () => { + const { exec, seen } = recordingExec(() => ONE_ROW); + + const result = await readTablePresence(exec, { table: TABLE, client: 'better-sqlite3', fallbackSql: FALLBACK_SQL }); + + expect(result).toEqual({ verdict: 'present', probe: 'catalog' }); + // ⭐ The card's whole point: on a recognised dialect the seam never sees the + // statement whose refusal the driver logs. + expect(seen).toEqual([buildTablePresenceSql(TABLE, 'better-sqlite3')]); + expect(seen.join('\n')).not.toContain('WHERE 1 = 0'); + }); + + it('zero rows means ABSENT — in all three dialect result-set spellings, and with no detail', async () => { + const empties: Array<[string, unknown]> = [ + ['better-sqlite3', []], + ['pg', { rows: [], rowCount: 0 }], + ['mysql2', [[], []]], + ]; + for (const [client, empty] of empties) { + const { exec, seen } = recordingExec(() => empty); + + const result = await readTablePresence(exec, { table: TABLE, client, fallbackSql: FALLBACK_SQL }); + + expect(result).toEqual({ verdict: 'absent', probe: 'catalog' }); + expect(result.detail).toBeUndefined(); + expect(seen.join('\n')).not.toContain('WHERE 1 = 0'); + } + }); + + it('a non-empty result set in each dialect spelling means PRESENT', async () => { + const filled: Array<[string, unknown]> = [ + ['better-sqlite3', ONE_ROW], + ['pg', { rows: ONE_ROW, rowCount: 1 }], + ['mysql2', [ONE_ROW, []]], + ]; + for (const [client, rows] of filled) { + const { exec } = recordingExec(() => rows); + const result = await readTablePresence(exec, { table: TABLE, client, fallbackSql: FALLBACK_SQL }); + expect(result.verdict).toBe('present'); + } + }); + + it('a seam that answers NOTHING is no-answer — #10789, unchanged and still separated by detail', async () => { + const { exec } = recordingExec(() => null); + + const result = await readTablePresence(exec, { table: TABLE, client: 'pg', fallbackSql: FALLBACK_SQL }); + + expect(result.verdict).toBe('no-answer'); + expect(result.detail).toMatch(/no result set/); + }); + + it('⛔ THE FENCE: a refused catalog statement is UNREADABLE, never absent', async () => { + // A mis-compiled arm, a permission denial and a dropped connection all land + // here. Reading any of them as "the table is not there" is what would make + // the repair decline in silence on an unexercised dialect. + for (const refusal of [ + new Error('near "to_regclass": syntax error'), + new Error('permission denied for table pg_class'), + new Error('ECONNREFUSED 127.0.0.1:5432'), + ]) { + const { exec } = recordingExec(() => refusal); + + const result = await readTablePresence(exec, { table: TABLE, client: 'pg', fallbackSql: FALLBACK_SQL }); + + expect(result.verdict).toBe('unreadable'); + expect(result.verdict).not.toBe('absent'); + expect(result.probe).toBe('catalog'); + expect(result.detail).toContain(refusal.message); + } + }); + + it('⛔ even a refusal whose words LOOK like absence is unreadable on the catalog arm', async () => { + // The catalog statement does not read from the target table, so a phrase + // naming it cannot be evidence about it — it is evidence that something + // else is wrong. ⛔ Do not add an `isMissingTableError` branch here: it + // would re-open the exact conflation this card closes, one layer down. + const { exec } = recordingExec(() => new Error(`no such table: ${TABLE}`)); + + const result = await readTablePresence(exec, { table: TABLE, client: 'better-sqlite3', fallbackSql: FALLBACK_SQL }); + + expect(result.verdict).toBe('unreadable'); + }); +}); + +describe('[#17175] readTablePresence — the fallback arm, on a dialect with no catalog statement', () => { + const UNKNOWN = { table: TABLE, client: 'oracledb', fallbackSql: FALLBACK_SQL }; + + it('runs the CALLER\'s own statement — not one invented here', async () => { + const { exec, seen } = recordingExec(() => []); + + const result = await readTablePresence(exec, UNKNOWN); + + expect(seen).toEqual([FALLBACK_SQL]); + expect(result).toEqual({ verdict: 'present', probe: 'fallback' }); + }); + + it('a refusal this table\'s own absence explains is ABSENT — the pre-#17175 answer, kept', async () => { + const { exec } = recordingExec(() => new Error(`no such table: ${TABLE}`)); + + const result = await readTablePresence(exec, UNKNOWN); + + expect(result).toEqual({ verdict: 'absent', probe: 'fallback' }); + }); + + it('⛔ a refusal naming a DIFFERENT relation is unreadable — #13324 narrowing, not absence', async () => { + const { exec } = recordingExec(() => new Error('no such table: some_other_table')); + + const result = await readTablePresence(exec, UNKNOWN); + + expect(result.verdict).toBe('unreadable'); + expect(result.verdict).not.toBe('absent'); + }); + + it('⛔ THE FENCE on this arm too: a refusal that is not about absence is unreadable', async () => { + const { exec } = recordingExec(() => new Error('ECONNREFUSED 127.0.0.1:3306')); + + const result = await readTablePresence(exec, UNKNOWN); + + expect(result.verdict).toBe('unreadable'); + expect(result.detail).toContain('ECONNREFUSED'); + }); + + it('a seam that answers nothing is no-answer here too', async () => { + const { exec } = recordingExec(() => undefined); + + const result = await readTablePresence(exec, UNKNOWN); + + expect(result.verdict).toBe('no-answer'); + expect(result.detail).toMatch(/no result set/); + }); + + it('POSITIVE CONTROL: the same seam on a RECOGNISED dialect takes the other arm', async () => { + // Without this, every assertion above would still pass if + // `catalogFamilyOf` had silently stopped recognising anything — the whole + // file would be testing the fallback and reporting success. + const { exec, seen } = recordingExec((sql) => (sql.includes('sqlite_master') ? ONE_ROW : [])); + + const result = await readTablePresence(exec, { ...UNKNOWN, client: 'better-sqlite3' }); + + expect(result.probe).toBe('catalog'); + expect(seen).not.toContain(FALLBACK_SQL); + }); +}); diff --git a/packages/metadata-protocol/src/migrations/read-probe.testkit.ts b/packages/metadata-protocol/src/migrations/read-probe.testkit.ts new file mode 100644 index 0000000000..49b04268ce --- /dev/null +++ b/packages/metadata-protocol/src/migrations/read-probe.testkit.ts @@ -0,0 +1,53 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#17175] Seam-double support: recognise the shared presence probe. + * + * The `kernel:ready` migrations used to ask "is this table here?" with a + * statement that could only answer "no" by being REFUSED, so a seam double said + * "the table is there" by simply not throwing, and every double in this package + * dispatched on the substring `WHERE 1 = 0`. Since #17175 the question is asked + * of the CATALOG, and a catalog that returns zero rows means ABSENT — so a + * double that falls through to its `return []` now says the table is gone. + * + * ⇒ A double that means "present" has to answer the catalog statement with a + * ROW, and this is how it recognises one. Derived from + * {@link buildTablePresenceSql} rather than pasted, so an arm whose text changes + * moves every double with it instead of leaving one quietly answering nothing. + * + * ⛔ Test support only. Nothing under `src/index.ts` imports it, so it is not + * bundled, and it carries no `vitest` import so the assertions stay in the + * fixture that owns them. + */ + +import { buildTablePresenceSql } from './read-probe.js'; + +/** + * Every knex client spelling `read-probe.ts` compiles a catalog arm for — the + * union of its three families, so a double recognises the probe whatever + * dialect the test names. + */ +const CATALOG_CLIENTS: readonly string[] = [ + 'sqlite3', + 'sqlite', + 'better-sqlite3', + 'postgres', + 'pg', + 'postgresql', + 'pgnative', + 'mysql', + 'mysql2', +]; + +/** Is `sql` the catalog presence statement for `table`, on any supported dialect? */ +export function isTablePresenceCatalogSql(sql: string, table: string): boolean { + return CATALOG_CLIENTS.some((client) => buildTablePresenceSql(table, client) === sql); +} + +/** + * The row a double returns to mean "yes, that table is here". + * + * The column name is irrelevant — every arm projects a bare `1` and the probe + * only counts rows — so this is one row of anything. + */ +export const TABLE_IS_PRESENT_ROWS: Record[] = [{ present: 1 }]; diff --git a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.live-mysql.test.ts b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.live-mysql.test.ts index 9c7b46b4ee..5d9a7aefac 100644 --- a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.live-mysql.test.ts +++ b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.live-mysql.test.ts @@ -62,6 +62,7 @@ import { type SeedTenancySeam, } from './seed-tenancy-backfill.js'; import { currentLiveMysqlDatabase } from './live-mysql-database.testkit.js'; +import { buildTablePresenceSql, readTablePresence } from './read-probe.js'; const MYSQL_URL = process.env.OS_TEST_MYSQL_URL; const EXPECT_LIVE = process.env.OS_EXPECT_LIVE_DIALECT_MATRIX === '1'; @@ -194,7 +195,12 @@ describe.skipIf(!MYSQL_URL)('#9381 seed-tenancy backfill on a LIVE MySQL', () => await seedFixture(); const client = 'mysql2'; const statements: Array<[string, string, unknown[]]> = [ - ['presence probe', buildSequencesPresenceSql(client), []], + ['presence probe (fallback arm)', buildSequencesPresenceSql(client), []], + // [#17175] The statement the boot path actually runs now. It is in this + // list for the same reason every other one is: a parse error on MySQL is + // invisible to a SQLite-only run, and this arm's failure mode is worse + // than a parse error — it is caught and read as "the table is not there". + ['presence probe (catalog arm)', buildTablePresenceSql(SEQUENCES_TABLE, client) as string, []], ['split probe', buildSplitProbeSql(client), [GLOBAL_TENANT, GLOBAL_TENANT]], ['organization probe', buildOrganizationProbeSql(client), []], ['collision probe', buildCollisionProbeSql(OBJECT, FIELD, client), []], @@ -212,6 +218,60 @@ describe.skipIf(!MYSQL_URL)('#9381 seed-tenancy backfill on a LIVE MySQL', () => } }); + it('[#17175] the catalog presence probe ANSWERS on MySQL — both directions, on the live server', async () => { + await seedFixture(); + + // Present: the fixture created the counter table, so the catalog names it. + const present = await readTablePresence( + (sql: string) => conn.query(sql) as Promise, + { table: SEQUENCES_TABLE, client: 'mysql2', fallbackSql: buildSequencesPresenceSql('mysql2') }, + ); + expect(present).toEqual({ verdict: 'present', probe: 'catalog' }); + + // ⭐ Absent: a table this database does not have. The probe must ANSWER + // rather than raise — on MySQL that means `information_schema.tables` + // returning zero rows, which is the assertion a SQLite-only run cannot make. + const absent = await readTablePresence( + (sql: string) => conn.query(sql) as Promise, + { + table: 'os17175_absent_table', + client: 'mysql2', + fallbackSql: 'SELECT 1 FROM os17175_absent_table WHERE 1 = 0', + }, + ); + expect(absent).toEqual({ verdict: 'absent', probe: 'catalog' }); + + // ⛔ And the control that makes the line above mean something: the fallback + // statement this probe did NOT run is one the server really does refuse, so + // "answered zero rows" is a reading about the catalog arm and not about a + // table that happens to exist. + await expect(conn.query('SELECT 1 FROM os17175_absent_table WHERE 1 = 0')).rejects.toThrow(); + }); + + it('[#17175] the scope is the CONNECTED schema — a same-named table elsewhere is not this one', async () => { + // `information_schema.tables` without `table_schema = DATABASE()` sees every + // schema on the server, so the arm would answer "present" for a table in a + // database this connection is not using. Measured here rather than argued. + const other = `${DB}_17175_other`; + await conn.query(`CREATE DATABASE IF NOT EXISTS \`${other}\``); + try { + await conn.query(`CREATE TABLE IF NOT EXISTS \`${other}\`.os17175_elsewhere (id INT)`); + + const verdict = await readTablePresence( + (sql: string) => conn.query(sql) as Promise, + { + table: 'os17175_elsewhere', + client: 'mysql2', + fallbackSql: 'SELECT 1 FROM os17175_elsewhere WHERE 1 = 0', + }, + ); + + expect(verdict.verdict).toBe('absent'); + } finally { + await conn.query(`DROP DATABASE IF EXISTS \`${other}\``); + } + }); + it('a multi-autonumber object stamps with one derived table per guard', async () => { await seedFixture(); await conn.query(`ALTER TABLE \`${OBJECT}\` ADD COLUMN \`ticket_no\` VARCHAR(64)`); diff --git a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.null-seam.test.ts b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.null-seam.test.ts index 150fb776d0..1e0136f6e9 100644 --- a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.null-seam.test.ts +++ b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.null-seam.test.ts @@ -69,8 +69,10 @@ import { resolveSeedTenancySeam, GLOBAL_TENANT, ORGANIZATION_TABLE, + SEQUENCES_TABLE, } from './seed-tenancy-backfill.js'; import type { SeedTenancyExec } from './seed-tenancy-backfill.js'; +import { TABLE_IS_PRESENT_ROWS, isTablePresenceCatalogSql } from './read-probe.testkit.js'; function createLogger() { return { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; @@ -89,10 +91,19 @@ const nonAnsweringSeam: SeedTenancyExec = async () => null; * A seam that ANSWERS every probe, with an empty result set in one dialect's * spelling. A real install with nothing to repair looks exactly like this: the * counter table exists, and no object holds counters on both sides of a split. + * + * [#17175] "The counter table exists" is now SAID rather than implied. It used + * to be implied by not throwing, because the presence probe could only answer + * "no" by being refused; the catalog probe answers "no" with zero rows, so a + * seam that returns an empty set to EVERY statement is now a seam saying the + * table is gone. The empty-set spelling under test is unchanged — it is still + * what every other probe gets, which is what keeps this fixture a pin on + * `isResultSet` rather than on the presence probe. */ function answeringEmptySeam(spelling: 'sqlite' | 'pg' | 'mysql'): SeedTenancyExec { const empty = { sqlite: [], pg: { rows: [], rowCount: 0 }, mysql: [[], []] }[spelling]; - return async () => empty; + return async (sql: string) => + isTablePresenceCatalogSql(sql, SEQUENCES_TABLE) ? TABLE_IS_PRESENT_ROWS : empty; } // ─────────────────────────────────────────────────────────────────────────── @@ -173,7 +184,8 @@ describe('#10789 a seam that returns no result set is ABSENT, not empty', () => // Pre-fix this is also `no-split`, for the same reason and one probe later. const result = await backfillSeedTenancy( { - exec: async (sql: string) => (sql.includes('WHERE 1 = 0') ? [] : null), + exec: async (sql: string) => + isTablePresenceCatalogSql(sql, SEQUENCES_TABLE) ? TABLE_IS_PRESENT_ROWS : null, client: 'better-sqlite3', }, createLogger() as any, @@ -263,6 +275,7 @@ describe('#10789 a seam that answers with no rows still reports no-split', () => } return { affectedRows: 3 }; // not a result set, by design } + if (isTablePresenceCatalogSql(sql, SEQUENCES_TABLE)) return TABLE_IS_PRESENT_ROWS; if (sql.includes('WHERE 1 = 0')) return []; if (sql.includes('LEFT JOIN')) { return [ @@ -302,7 +315,7 @@ describe('#10789 the branches this fix must leave alone', () => { expect(result.status).toBe('no-driver'); }); - it('[non-effect] a seam that THROWS is unchanged — that path was never the defect', async () => { + it('[#17175 supersedes] a seam that THROWS is now UNREADABLE, not absent', async () => { // Throwing is a driver present and refusing LOUDLY, and step 1's `catch` // already reported it honestly as `absent`. Only a seam that RETURNS a // non-answer was invisible, so only that one changed. @@ -316,15 +329,30 @@ describe('#10789 the branches this fix must leave alone', () => { createLogger() as any, ); - expect(result.status).toBe('absent'); - // No `detail` from the non-answer branch: this one did not take it. - expect(result.detail).toBeUndefined(); + // [#17175] This assertion USED to read `absent`, and the comment above it + // used to say a throwing seam "already reported it honestly". Measured + // against the presence probe this file pins, that was only half true: the + // throw it described was the probe's own `SELECT … WHERE 1 = 0` being + // REFUSED because the table is missing, which is an answer. A connection + // refusal is not, and folding the two together is what let this repair + // decline in silence on a seam that never looked. + // + // The catalog probe cannot be refused for the reason the old one was — it + // does not name the counter table in a FROM clause at all — so any throw on + // that arm is a genuine fault, and it is reported. What is preserved, and is + // pinned in `read-probe.test.ts`, is the FALLBACK arm: on a dialect with no + // catalog statement, a refusal that `isMissingTableError` recognises still + // reads `absent`. + expect(result.status).toBe('unreadable'); + expect(result.status).not.toBe('absent'); + expect(result.detail).toMatch(/ECONNREFUSED/); }); it('[non-effect] the split probe throwing still reports absent with the driver message', async () => { const result = await backfillSeedTenancy( { exec: async (sql: string) => { + if (isTablePresenceCatalogSql(sql, SEQUENCES_TABLE)) return TABLE_IS_PRESENT_ROWS; if (sql.includes('WHERE 1 = 0')) return []; throw new Error('no such table: _objectstack_sequences'); }, @@ -342,6 +370,7 @@ describe('#10789 the branches this fix must leave alone', () => { // guards, so a fix that rejected a legitimate answer would silently stop // this branch from ever running. const exec: SeedTenancyExec = async (sql: string) => { + if (isTablePresenceCatalogSql(sql, SEQUENCES_TABLE)) return TABLE_IS_PRESENT_ROWS; if (sql.includes('WHERE 1 = 0')) return []; if (sql.includes('LEFT JOIN')) { return [ diff --git a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.test.ts b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.test.ts index b96fd65d62..8e3cc3833e 100644 --- a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.test.ts +++ b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.test.ts @@ -50,6 +50,7 @@ import { ORGANIZATION_TABLE, } from './seed-tenancy-backfill.js'; import type { SeedTenancyBackfillResult } from './seed-tenancy-backfill.js'; +import { TABLE_IS_PRESENT_ROWS, isTablePresenceCatalogSql } from './read-probe.testkit.js'; describe('#8686 normalizeRows — one reader for three dialect shapes', () => { const rows = [{ object: 'crm_case', field: 'case_number' }]; @@ -405,7 +406,12 @@ describe('#12394 the counter handoff writes the row the driver will read', () => const rows = new Map>(); for (const row of seed) rows.set(String(row.key_hash), { ...row }); const exec = async (sql: string, params: unknown[] = []): Promise => { - if (sql.includes('WHERE 1 = 0')) return []; // presence + key-shape probes + // [#17175] The presence question is asked of the catalog now, and a + // catalog answering zero rows means ABSENT — so "the counter table is + // there" has to be said with a ROW. The `WHERE 1 = 0` line below is still + // live: it is the key-SHAPE probe, which still asks by being refused. + if (isTablePresenceCatalogSql(sql, SEQUENCES_TABLE)) return TABLE_IS_PRESENT_ROWS; + if (sql.includes('WHERE 1 = 0')) return []; // key-shape probe if (sql.includes('LEFT JOIN')) { return [ { object: OBJECT, field: FIELD, global_last_value: 38, organization_last_value: 1 }, @@ -537,7 +543,8 @@ describe('#9451 the seed-tenancy repair leaves a durable receipt', () => { // Dispatched on the statements the module actually compiles, so a builder // that changed shape breaks this fixture rather than silently turning it // into a healthy install. - if (sql.includes('WHERE 1 = 0')) return []; // presence + key-shape probes + if (isTablePresenceCatalogSql(sql, SEQUENCES_TABLE)) return TABLE_IS_PRESENT_ROWS; + if (sql.includes('WHERE 1 = 0')) return []; // key-shape probe if (sql.includes('LEFT JOIN')) { return [ { @@ -680,7 +687,8 @@ describe('#9451 the seed-tenancy repair leaves a durable receipt', () => { const log = createLogger(); const result = await backfillSeedTenancy( { - exec: async (sql: string) => (sql.includes('SELECT 1') ? [] : []), + exec: async (sql: string) => + isTablePresenceCatalogSql(sql, SEQUENCES_TABLE) ? TABLE_IS_PRESENT_ROWS : [], client: 'better-sqlite3', ledger: store.ledger, }, @@ -769,6 +777,7 @@ describe('#9451 the seed-tenancy repair leaves a durable receipt', () => { const result = await backfillSeedTenancy( { exec: async (sql: string) => { + if (isTablePresenceCatalogSql(sql, SEQUENCES_TABLE)) return TABLE_IS_PRESENT_ROWS; if (sql.includes('LEFT JOIN')) { return [ { object: 'sys_migration', field: 'seq', global_last_value: 7, organization_last_value: 2 }, @@ -842,6 +851,7 @@ describe('#12395 zero organizations is a third state, not the ambiguous one', () const sql: string[] = []; const exec = async (statement: string) => { sql.push(statement); + if (isTablePresenceCatalogSql(statement, SEQUENCES_TABLE)) return TABLE_IS_PRESENT_ROWS; if (statement.includes('WHERE 1 = 0')) return []; if (statement.includes('LEFT JOIN')) { return [ diff --git a/packages/runtime/src/seed-tenancy-autonumber-split.integration.test.ts b/packages/runtime/src/seed-tenancy-autonumber-split.integration.test.ts index 48e4bf946a..364727e897 100644 --- a/packages/runtime/src/seed-tenancy-autonumber-split.integration.test.ts +++ b/packages/runtime/src/seed-tenancy-autonumber-split.integration.test.ts @@ -578,3 +578,131 @@ describe('#8686 seed/API tenancy split — autonumber scope', () => { expect(await countUntenanted(driver)).toBe(0); }); }); + +/** + * [#17175] The boot-path presence probe stops making the driver shout — and the + * driver keeps shouting about everything else. + * + * ## Why this block is HERE and not in `metadata-protocol` + * + * The whole defect lives between two correct components. The migration is right + * to treat a missing counter table as "no"; the driver is right to write a + * refused raw statement to the operator's log, because it has no way to know the + * caller expected the refusal (the raw path carries no table identity — see + * `read-probe.ts`'s header). The visible failure is the PAIR, so only a real + * `SqlDriver` over a real database can show it: a fixture would have to encode + * the very log behaviour it is meant to measure. + * + * ## The two halves, and the second is the one that matters + * + * ⭐ A test proving the line is gone, without proving real errors survive, is a + * test for the wrong thing — it would pass just as well over a driver whose + * error channel had been muted wholesale, which is the repair the ruling on this + * card explicitly refused. So both are asserted against the SAME driver, the + * SAME log sink and the SAME raw path, in the same test run. + */ +describe('#17175 a normal boot stops printing DATABASE_ERROR for an expected miss', () => { + /** Every line the driver's default log channels emit, in order. */ + function captureDriverLog(driver: any): { lines: string[]; restore: () => void } { + const lines: string[] = []; + const original = driver.logger; + driver.logger = { + warn: (msg: string) => lines.push(`warn: ${msg}`), + error: (msg: string) => lines.push(`error: ${msg}`), + info: (msg: string) => lines.push(`info: ${msg}`), + }; + return { lines, restore: () => { driver.logger = original; } }; + } + + it('⭐ the expected miss is SILENT — and a real refusal on the same path is not', async () => { + const { driver, engine } = await bootInstall(); + + // The card's condition: a database with no counter table. ⚠️ Measured + // rather than assumed — `initObjects` on an object that DECLARES an + // autonumber provisions `_objectstack_sequences` up front on this driver, + // so it is dropped here instead of being expected never to have existed. + // The consumer's install reaches the same state by never declaring one. + await (driver as any).knex.schema.dropTableIfExists('_objectstack_sequences'); + await expect( + (driver as any).knex('_objectstack_sequences').select('tenant_id'), + ).rejects.toThrow(/no such table/); + + const seam = resolveSeedTenancySeam(engine); + const { lines, restore } = captureDriverLog(driver); + try { + const result = await backfillSeedTenancy(seam, createLogger() as any); + + // The answer is unchanged: the table is not there, and the migration says + // so. ⛔ Not `unreadable` — an ANSWERED absence is still an answer. + expect(result.status).toBe('absent'); + expect(result.detail).toBeUndefined(); + + // ⭐ THE CARD. Before this change the line below was present, once per + // boot, on stderr, naming `_objectstack_sequences` and `no such table`. + expect(lines.filter((l) => l.includes('DATABASE_ERROR'))).toEqual([]); + expect(lines.join('\n')).not.toContain('no such table'); + expect(lines.join('\n')).not.toContain('_objectstack_sequences'); + + // ⭐ THE HALF THAT MATTERS. Same driver, same sink, same raw terminal: a + // statement the backend genuinely refuses is still written to the log in + // full. A repair that silenced this would have satisfied every assertion + // above and broken the thing the assertions exist to protect. + await expect( + driver.execute('SELECT 1 FROM os17175_really_absent WHERE 1 = 0'), + ).rejects.toThrow(); + + const loud = lines.filter((l) => l.includes('DATABASE_ERROR')); + expect(loud).toHaveLength(1); + expect(loud[0]).toContain('os17175_really_absent'); + expect(loud[0]).toContain('no such table'); + // The level is the driver's own, unchanged by this card: `warn`, which is + // what `console.warn` writes to stderr on a host that installs no logger. + expect(loud[0].startsWith('warn: ')).toBe(true); + } finally { + restore(); + } + }); + + it('the probe the driver now runs never reads FROM the counter table', async () => { + // The mechanism, taken from the statement the seam is actually handed rather + // than from the builder: a probe that does not name the table in a FROM + // clause cannot be refused because the table is missing. + const { driver, engine } = await bootInstall(); + await (driver as any).knex.schema.dropTableIfExists('_objectstack_sequences'); + const seam = resolveSeedTenancySeam(engine); + const seen: string[] = []; + const watched = { + exec: async (sql: string, params?: unknown[]) => { + seen.push(sql); + return seam!.exec(sql, params); + }, + client: seam!.client, + ledger: (seam as any).ledger, + }; + + await backfillSeedTenancy(watched as any, createLogger() as any); + + expect(seen).toHaveLength(1); + expect(seen[0]).toContain('sqlite_master'); + expect(seen[0]).not.toContain('WHERE 1 = 0'); + expect(seen[0]).not.toContain('FROM "_objectstack_sequences"'); + // Control: the statement it replaced IS refused by this very database, so + // "no line was logged" above is a reading about the new probe and not about + // a database that would have tolerated the old one. + await expect(driver.execute(buildSequencesPresenceSql(seam!.client))).rejects.toThrow(); + }); + + it('an install that HAS the counter table still reaches the repair', async () => { + // The other direction of the same probe: a catalog arm that answered + // "absent" for a table that is there would make this migration a permanent + // no-op, which is the silent failure the ruling on this card fenced against. + const { engine } = await bootInstall(); + await createOrganization(engine); + await apiCreate(engine, 'api 1'); + + const result = await backfillSeedTenancy(resolveSeedTenancySeam(engine), createLogger() as any); + + expect(result.status).toBe('no-split'); + expect(result.status).not.toBe('absent'); + }); +}); From 21c11682ce9fbf72348b77a8f16750aeed5a23f9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 03:06:06 +0000 Subject: [PATCH 3/4] feat(metadata-protocol): one shared non-raising table-presence probe for the kernel:ready migrations Claude-Session: https://claude.ai/code/session_01RuoNSXUbBoWHkNS4AknTrM Co-authored-by: Claude --- .../17175-non-raising-table-presence-probe.md | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 .changeset/17175-non-raising-table-presence-probe.md diff --git a/.changeset/17175-non-raising-table-presence-probe.md b/.changeset/17175-non-raising-table-presence-probe.md new file mode 100644 index 0000000000..4b5a313531 --- /dev/null +++ b/.changeset/17175-non-raising-table-presence-probe.md @@ -0,0 +1,81 @@ +--- +'@objectstack/metadata-protocol': minor +--- + +The `kernel:ready` migrations ask whether a table exists WITHOUT running a statement that has to be refused, so a normal boot stops printing `[sql-driver] DATABASE_ERROR … no such table` (#17175) + +Two migrations on the boot hook asked "does this table exist?" with a statement +that cannot succeed when the answer is no — `SELECT "tenant_id" FROM +"_objectstack_sequences" WHERE 1 = 0` in `seed-tenancy-backfill.ts`, and `SELECT +1 FROM sys_setting WHERE 1 = 0` in `sys-setting-identity-index.ts` — and read +the refusal as "no". Both are correct on their own terms. Both make +`SqlDriver.execute()`'s raw terminal write the statement and the dialect's +message to the operator's log on the way out. + +Measured on this tree against real `better-sqlite3`: exactly one line per probe, +on `console.warn` — i.e. **stderr** — carrying both the `DATABASE_ERROR` token +and `no such table`. It fires on **every boot** of every install that has never +allocated an autonumber, and again on every boot of every kernel that does not +register the optional `service-settings`. + +⭐ The cost is not the line. It is that operators learn this product prints +errors when nothing is wrong, and then miss the one that matters. A consumer told +to read the boot log (`objectstack-ai/hotclm`'s `AGENTS.md` names `no such table` +as a failing boot) must either ignore an unactionable ERROR every boot or chase a +platform-internal probe. + +**The question is now asked of the CATALOG.** A new shared +`migrations/read-probe.ts` compiles one arm per dialect family — `sqlite_master` +for SQLite, `to_regclass` for Postgres, `information_schema.tables` scoped with +`DATABASE()` for MySQL — each of which returns zero rows for a table that is not +there instead of being refused. Both migrations call it; the probe lives once, +not once per site. + +**⛔ Why not in the driver.** Quietening a refusal requires classifying it, this +repo has one predicate for that (`isMissingTableError`), and it needs the name of +the thing the caller was reading — which the raw path structurally does not have +(`rawStatementFaultError` declares no targeted table, and +`driver-error-classification.callers.test.ts` fails any in-repo call that omits +`readObject`). An unclassified demotion of the driver's raw terminal would +quieten real failures too. The caller knows the table; the driver does not. + +**⛔ The fence, and it is the one way this repair can go wrong.** A catalog arm +mis-compiled for some dialect would be refused, caught by the same `catch` the +expected miss uses, and read as "the table is not there" — turning a stored-row +data repair into a silent no-op on whichever dialect nobody exercised. So the +probe answers four verdicts rather than a boolean, and `'unreadable'` is never +folded into `'absent'`: it is returned, and reported at `warn`. An unrecognised +dialect gets no guessed catalog statement at all — it keeps the caller's own +`WHERE 1 = 0` probe, whose refusal is now *classified* with +`isMissingTableError(error, table)` rather than swallowed as absence. + +**Why `minor`.** + +- `SeedTenancyBackfillStatus` gains `'unreadable'`. It is an OUTPUT union, so no + input a caller writes is affected; the one consumer shape that could break is + an exhaustive `switch` with a `never` default, which is why this is not a + `patch`. +- `ensureSysSettingIdentityIndex` gains an optional third parameter + (`{ client? }`). Callers that pass two arguments are unchanged and keep + today's behaviour exactly — without a client there is no catalog arm and the + pre-existing probe runs. +- `buildSequencesPresenceSql` and `buildSysSettingPresenceSql` are unchanged in + text and still exported. They are no longer what the boot path runs first. +- `isResultSet` and `normalizeRows` moved to `migrations/read-probe.ts` and are + re-exported from `seed-tenancy-backfill.ts` unchanged, so the package index and + every importer see no difference. + +**What did NOT change.** #10789's ruling stands: a seam that accepts a statement +and returns no result set still reports `absent` with the `detail` that separates +it. The driver's error channel is untouched — a statement the backend genuinely +refuses is still written to the log in full, asserted against the same driver and +the same sink in the same test as the silence. + +**Dialect coverage, stated rather than implied.** The SQLite arm is pinned end to +end against a real `SqlDriver` (`packages/runtime`'s +`seed-tenancy-autonumber-split.integration.test.ts`); the MySQL arm runs against +the live server in `seed-tenancy-backfill.live-mysql.test.ts`, in both directions +and with the connected-schema scope measured. ⛔ The **Postgres** arm is NOT +MEASURED against a live server: this package has no live-PG harness, no `pg` +dependency, and its CI leg supplies `OS_TEST_MYSQL_URL` only while filtering to +`live-mysql`. Its statement text is pinned; running it is not. From 21db91bf5d1e3c0efbd6bc63bba95df12fd2f2be Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 03:29:32 +0000 Subject: [PATCH 4/4] fix(metadata-protocol): keep tracker ids out of the operator-facing probe reports Claude-Session: https://claude.ai/code/session_01RuoNSXUbBoWHkNS4AknTrM Co-authored-by: Claude --- .../src/migrations/seed-tenancy-backfill.ts | 7 +++++-- .../src/migrations/sys-setting-identity-index.ts | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts index 4465f0bd18..9598f1be47 100644 --- a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts +++ b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts @@ -1235,11 +1235,14 @@ export async function backfillSeedTenancy( fallbackSql: buildSequencesPresenceSql(client), }); if (presence.verdict === 'unreadable') { + // [#8686, #17175] ⛔ The ids stay in this comment and out of the string: a + // runtime line reaches operators, who have no tracker to resolve them with + // (`check:doc-authoring`). logger?.warn?.( - `[metadata-protocol] the seed/API tenancy repair (#8686) could not read whether ` + + `[metadata-protocol] the seed/API tenancy repair could not read whether ` + `"${SEQUENCES_TABLE}" exists, so it did NOT run and nothing was changed. This is not the ` + `table being absent — that answer is silent and normal. Verify by hand with: ` + - `${buildSequencesPresenceSql(client)} (#17175).`, + `${buildSequencesPresenceSql(client)}`, { error: presence.detail, probe: presence.probe }, ); return { status: 'unreadable', ...empty, detail: presence.detail }; diff --git a/packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts b/packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts index 895f4aaf43..5b06d0d08f 100644 --- a/packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts +++ b/packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts @@ -499,12 +499,15 @@ export async function ensureSysSettingIdentityIndex( // [#17175] Not 'absent'. The tightening did not run, the previous index // is kept, and — unlike absence, which is normal and silent — an // operator is told, because nothing here looked at anything. + // [#8629, #17175] ⛔ The ids stay in this comment and out of the string: + // a runtime line reaches operators, who have no tracker to resolve them + // with (`check:doc-authoring`). logProblem( logger, `[metadata-protocol] could not read whether "${SYS_SETTING_TABLE}" exists, so the row-identity ` + - `index tightening (#8629) did NOT run and the table keeps whatever unique index it had. ` + + `index tightening did NOT run and the table keeps whatever unique index it had. ` + `⛔ This is not the table being absent, which is a normal, silent no-op on a kernel without ` + - `service-settings. Verify by hand with: ${buildSysSettingPresenceSql()} (#17175).`, + `service-settings. Verify by hand with: ${buildSysSettingPresenceSql()}`, presence.detail ?? '', ); return { status: 'failed', detail: presence.detail };