diff --git a/.changeset/driver-sql-object-def-param-keys.md b/.changeset/driver-sql-object-def-param-keys.md new file mode 100644 index 0000000000..3078cca521 --- /dev/null +++ b/.changeset/driver-sql-object-def-param-keys.md @@ -0,0 +1,13 @@ +--- +"@objectstack/driver-sql": minor +--- + +`SqlDriver`'s object-definition parameters now DECLARE every key they read. `initObjects` accepts `lifecycle`, and the whole rotation chain — `rotateShards`, `ensureRotation`, `ensureShardTable` — accepts `tenancy` and `indexes`, spelled as a **fresh object literal** rather than only as a value bound to a variable first. + +The driver read those keys off caller objects all along, through `(obj as any).`, while the parameter's own inline type listed none of them. That is refused or accepted depending only on where the object is spelled: TypeScript's excess-property check fires on a fresh literal and not on one hoisted to a variable, so the same call compiles in one shape and is `TS2353` in the other. The loud outcome is the harmless one. The bad one is an author — or an AI reading the signature — concluding the key is not accepted and DROPPING it, at which point a declared UNIQUE is never synced and an ADR-0057 rotation policy is never armed, with nothing anywhere saying so. + +This is the third instance of one class, not a third coincidence: `tenancy` (#4311) and `indexes` (#16570) were the first two, each fixed one key at a time. The class is now held by a gate — `scripts/check-object-def-param-keys.mjs` — that reads parameter lists as an AST and covers the shape no in-file check could see: a subclass in another published package overriding one of these methods with a narrower literal. + +- **What widened.** `rotateShards(objectDef)` gains `tenancy?: any` and `indexes?: any[]`; `ensureRotation(…, obj, …)` gains the same two; `ensureShardTable(…, obj)` gains `indexes?: any[]`; `initObjects(objects)` gains `lifecycle?: any`. All three rotation links carry the keys, not just the leaf that reads them — declaring them only on the leaf would leave the two links above still narrowing the same value in flight, so a fresh literal handed to the public entry point would still have been refused. +- **What did NOT widen, deliberately.** The accept set still has a boundary: a misspelling (`indexs`, `tenancyy`, `lifecycl`) on a fresh literal is still `TS2353`, pinned by `@ts-expect-error` in `src/sql-driver-16711-object-def-param-keys.test.ts`. A "fix" that relaxed these parameters to `any`, or gave them an index signature, would have turned every other assertion green while deleting the entire layer of protection. +- **Four `as any` casts deleted**, including the residual one in `detectManagedDrift`, whose parameter had declared `indexes` all along. Behaviour is unchanged in every case — the keys were already being read. diff --git a/.changeset/driver-sqlite-wasm-inherits-object-def-keys.md b/.changeset/driver-sqlite-wasm-inherits-object-def-keys.md new file mode 100644 index 0000000000..eb3e399e6f --- /dev/null +++ b/.changeset/driver-sqlite-wasm-inherits-object-def-keys.md @@ -0,0 +1,9 @@ +--- +"@objectstack/driver-sqlite-wasm": minor +--- + +`SqliteWasmDriver.initObjects` accepts `tenancy`, `indexes` and `lifecycle` in a **fresh object literal**, inherited from the widened `SqlDriver` — and that inheritance is now asserted rather than assumed. + +This package overrides neither `initObjects` nor `registerObjectMetadata`, so its published `.d.ts` re-declares none of them and the door it exposes is `SqlDriver`'s, imported from `@objectstack/driver-sql`. Measured on the built declarations: zero re-declarations of `initObjects`, `registerObjectMetadata`, `rotateShards`, `ensureShardTable` or `registerManagedObjectMetadata`. That is the opposite direction of the defect the sibling packages carried — `TursoDriver` overrode `initObjects` with a narrower literal and shadowed a base-class fix for five weeks — and it is recorded here because a consumer reading only this package's changelog would otherwise never learn its accept set moved. + +`src/sqlite-wasm-16711-inherited-object-def-keys.test.ts` pins the inheritance inside this package's own tsc program: the inherited parameter is not `any`, each key is present on the element type, a fresh literal carrying them compiles and is read at run time, and a misspelling is still `TS2353`. It goes red both ways — if the base narrows again, and if a future override here re-declares the door more narrowly. diff --git a/.changeset/driver-turso-init-objects-declares-base-keys.md b/.changeset/driver-turso-init-objects-declares-base-keys.md new file mode 100644 index 0000000000..184e7dd2d2 --- /dev/null +++ b/.changeset/driver-turso-init-objects-declares-base-keys.md @@ -0,0 +1,13 @@ +--- +"@objectstack/driver-turso": minor +--- + +`TursoDriver.initObjects` now declares every key `SqlDriver.initObjects` declares — `tenancy`, `indexes` and `lifecycle` — so a caller of this package can spell them in a **fresh object literal** instead of hoisting the object to a variable to get past the type. + +`TursoDriver` OVERRIDES `initObjects`, and an override does not inherit the base's parameter type. Its own literal read `Array<{ name: string; fields?: Record }>`, which is what every consumer of `@objectstack/driver-turso` saw — so when #4311 declared `tenancy` on the base in August, that fix did not exist from outside this package, and stayed invisible for five weeks with nothing red anywhere. #16570's `indexes` fix would have escaped by the identical route. + +The type face was the only thing refusing the keys. The remote arm forwards the whole object through as `schema`, and `registerRemoteFieldMetadata` reads `tenancy` straight back off it, so the runtime carried both keys the entire time. `tenancy.enabled: false` is the key that decides whether a UNIQUE partitions globally or per organization — an author who hit the refusal and dropped it silently got the other answer. + +- `registerRemoteFieldMetadata(obj)` declares `tenancy?: any` and reads it directly; its `(obj as any).tenancy` cast is gone. +- The boundary is intact: a misspelling on a fresh literal is still `TS2353`, pinned in `src/turso-driver-16711-init-objects-param.test.ts`. +- `scripts/check-object-def-param-keys.mjs` now fails the build if this override — or any other subclass override in the workspace — declares fewer keys than the method it shadows, or erases the base's shape with an opaque type or an index signature. diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 7b78557dd9..a6ba3843af 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -4386,6 +4386,29 @@ jobs: - name: Check every driver runs the shared conformance cases run: pnpm check:driver-conformance + # Object-definition parameter keys (#16711). `SqlDriver` takes object + # definitions as inline object-literal parameter types and reads keys off + # them that the literal does not list, through `(obj as any).`. Three + # single-key cards fixed one key each (#4311 `tenancy`, #16570 `indexes`, + # #16711 `lifecycle`) before anyone called it a class. The escape is + # silent: TypeScript's excess-property check fires on a FRESH object + # literal and not on one bound to a variable first, so an author who hits + # the refusal drops the key — an unsynced UNIQUE, an unarmed ADR-0057 + # rotation policy — and nothing says so. + # + # ⛔ NOT scoped to sql-driver.ts, which is the whole ruling: `TursoDriver` + # OVERRIDES `initObjects` in a separately published package, so #4311's + # fix was invisible from outside `@objectstack/driver-sql` for five weeks + # and #16570's would have escaped identically. The gate compares every + # subclass override's declared keys against the base's, across packages, + # and refuses the two edits that would make that vacuous (an index + # signature, an opaque replacement type). AST-based, because the signature + # this class hides behind wraps across lines and a single-line grep for it + # returns a silence that reads exactly like a negative. + # Reads source files only; no build, ~2s. + - name: Check object-definition parameters declare the keys they are read for + run: pnpm check:object-def-param-keys + # Stall-guard self-test (#4250). scripts/run-with-stall-guard.mjs is what # turns a frozen Test Core into a labeled red; six jobs across five # workflows now route their test steps through it. But it only executes its diff --git a/package.json b/package.json index 0c50739f45..6da3c2313c 100644 --- a/package.json +++ b/package.json @@ -162,6 +162,7 @@ "check:type-check-debt": "node scripts/check-type-check-coverage.mjs --self-test && node scripts/check-type-check-coverage.mjs --re-measure", "check:driver-conformance": "node scripts/check-driver-conformance.mjs --self-test && node scripts/check-driver-conformance.mjs", "check:driver-memory-census": "node scripts/check-driver-memory-census.mjs --self-test && node scripts/check-driver-memory-census.mjs", + "check:object-def-param-keys": "node scripts/check-object-def-param-keys.mjs --self-test && node scripts/check-object-def-param-keys.mjs", "check:engine-double-contract": "node scripts/check-engine-double-contract.mjs --self-test && node scripts/check-engine-double-contract.mjs", "check:where-matcher": "node scripts/check-where-matcher-conformance.mjs --self-test && node scripts/check-where-matcher-conformance.mjs", "check:objectql-double-limit": "node scripts/check-objectql-double-limit.mjs --self-test && node scripts/check-objectql-double-limit.mjs", diff --git a/packages/drivers/driver-sql/src/sql-driver-11794-richtext-text-family.test.ts b/packages/drivers/driver-sql/src/sql-driver-11794-richtext-text-family.test.ts index 6d90f4040e..e1519cb2dc 100644 --- a/packages/drivers/driver-sql/src/sql-driver-11794-richtext-text-family.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-11794-richtext-text-family.test.ts @@ -322,8 +322,19 @@ for (const liveCell of [PG_CELL, MYSQL_CELL]) { // it physically too. Both directions measured, boundary included. const KT = `${T}_keyed`; // Hoisted (not an inline literal) the way #11374's `boundedObject()` - // is: `indexes` rides through `initObjects` beyond its narrow - // parameter type, exactly as the platform objects declare it. + // is, exactly as the platform objects declare it. + // + // ⚠️ The second half of what this comment used to say has EXPIRED and + // is kept here as a dated record rather than deleted: it read + // "`indexes` rides through `initObjects` beyond its narrow parameter + // type", and that was true — the signature declared no `indexes` and + // the driver read the key through an `as any` anyway. #16570 declared + // it and #16711 closed the class, so the hoist is no longer LOAD-BEARING + // here; an inline literal would compile today. It stays because + // mirroring #11374's authoring shape is why it was written that way in + // the first place, and because this suite is about column widths, not + // about parameter types. The pin that must stay inline is + // `sql-driver-16711-object-def-param-keys.test.ts`. const keyedObject = { name: KT, fields: { diff --git a/packages/drivers/driver-sql/src/sql-driver-15479-shadow-plain-unique-duplicates.test.ts b/packages/drivers/driver-sql/src/sql-driver-15479-shadow-plain-unique-duplicates.test.ts index e79de98437..0be5345118 100644 --- a/packages/drivers/driver-sql/src/sql-driver-15479-shadow-plain-unique-duplicates.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-15479-shadow-plain-unique-duplicates.test.ts @@ -125,13 +125,17 @@ declareDialectCell(MYSQL_CELL, 'hash-shadow plain unique over duplicates (#15479 ): Promise<{ logs: string[]; err: unknown }> => { driver = new SqlDriver(cell.config()); const logs = spy(); - await driver.initObjects([{ ...meta, indexes: [] }] as any); + // #16711: the `as any` that used to be on both of these calls was a + // workaround for `initObjects` not declaring `indexes`. The signature + // declares it now, so the cast is gone and these two calls are checked + // like any other. + await driver.initObjects([{ ...meta, indexes: [] }]); const knex = (driver as any).knex; await knex(meta.name).insert([ { id: 'a', ...row }, { id: 'b', ...row }, ]); - const err: unknown = await driver.initObjects([meta] as any).then( + const err: unknown = await driver.initObjects([meta]).then( () => null, (e) => e, ); @@ -228,7 +232,7 @@ declareDialectCell(MYSQL_CELL, 'hash-shadow plain unique over duplicates (#15479 it('still creates and enforces the plain shadow unique over clean data', async () => { driver = new SqlDriver(cell.config()); spy(); - await driver.initObjects([plainUniqueOn('os15479_clean')] as any); + await driver.initObjects([plainUniqueOn('os15479_clean')]); const { cols, idx } = await catalog('os15479_clean'); const shadow = cols.find((c: any) => isHashShadowColumn(c.COLUMN_NAME)); diff --git a/packages/drivers/driver-sql/src/sql-driver-16711-object-def-param-keys.test.ts b/packages/drivers/driver-sql/src/sql-driver-16711-object-def-param-keys.test.ts new file mode 100644 index 0000000000..1865e815f3 --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-16711-object-def-param-keys.test.ts @@ -0,0 +1,184 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#16711] The object-definition parameters on `SqlDriver` accept every key + * they READ, spelled as a **fresh object literal** — and still refuse a key + * that is genuinely not one of them. + * + * ## What this pins, and why it is a CLASS card rather than a third key + * + * `SqlDriver` reads keys off caller objects through `(obj as any).` while + * the parameter's own inline literal declares none of them. Three instances + * were carded and fixed one at a time before anyone called it a class: + * `tenancy` (#4311), `indexes` (#16570), and `lifecycle` here. Each fix left + * the next one standing, and each looked complete from inside its own card. + * + * The escape is silent by construction. TypeScript's excess-property check + * fires on a **fresh object literal** and not on one bound to a variable first: + * + * ```ts + * await driver.initObjects([{ ...bare, lifecycle: { storage: … } }]); // TS2353 + * const hoisted = { ...bare, lifecycle: { storage: … } }; + * await driver.initObjects([hoisted]); // accepted + * ``` + * + * so every existing caller happened to bind first and the package typechecked + * green for a reason unrelated to correctness. ⇒ a **variable-bound pin cannot + * go red on this defect**. Every call below is therefore an inline literal in + * argument position, which is what makes `tsc --noEmit` (this package's + * `typecheck` script) the instrument that measures it. + * + * ## §3 is the NEGATIVE CONTROL and it is the load-bearing section + * + * A "fix" that sets the parameter to `any`, or bolts an index signature onto + * it, turns §1 and §2 green **while deleting the entire layer of type + * protection they are about** — and nothing in a green run would say so. §3 + * asserts the other direction: a misspelling on a fresh literal still raises + * TS2353. The `@ts-expect-error` comments ARE the assertion — if any of those + * lines stops erroring, `tsc` fails the file with TS2578. + * + * ## The rotation chain is three links, not one + * + * `rotateShards` → `ensureRotation` → `ensureShardTable` all receive the SAME + * caller object, and only the leaf reads `indexes` / `tenancy`. Declaring the + * keys on the leaf alone would leave the two links above still narrowing the + * value in flight, so a fresh literal handed to the public `rotateShards` would + * still be refused. §2 calls the PUBLIC entry point for exactly that reason. + * + * Runs on the always-available in-memory SQLite cell — rotation is SQLite-only + * (`supportsRotation`), and the parameter types are not dialect-specific. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { SqlDriver } from './sql-driver.js'; +import { dialectCell } from './live-dialect-matrix.testkit.js'; + +const SQLITE = dialectCell('sqlite'); + +/** + * The base object, deliberately WITHOUT any of the keys under test — every call + * site spreads it and writes the key inline, so the literal being checked is + * fresh in argument position. + */ +const bareObject = (name: string) => ({ + name, + fields: { v: { type: 'text', maxLength: 64 } }, +}); + +const ROTATION = { storage: { strategy: 'rotation' as const, shards: 2, unit: 'day' as const } }; + +describe('SqlDriver object-definition parameters accept the keys they read (#16711)', () => { + let driver: SqlDriver | undefined; + afterEach(async () => { + await driver?.disconnect().catch(() => {}); + driver = undefined; + }); + + it('§1 initObjects: the inline `lifecycle` literal compiles AND arms ADR-0057 rotation', async () => { + const T = 'os16711_lifecycle'; + driver = new SqlDriver(SQLITE.config()); + + // Fresh literal in argument position — not hoisted to a variable first. + await driver.initObjects([{ ...bareObject(T), lifecycle: ROTATION }]); + + // The runtime half: the key is not merely ADMITTED by the type, it is still + // READ. Rotation replaces the base table with a read view over shard + // tables, so the physical shape says whether the policy was armed. A + // caller who dropped `lifecycle` at authoring time — the silent failure + // this card is about — would get an ordinary table and no shards, with + // nothing anywhere saying the declared policy went unimplemented. + // ⚠️ Read the whole catalog and filter HERE. A `LIKE '…\\_\\_r%'` predicate + // needs an explicit `ESCAPE` clause in SQLite, and without one it matches + // nothing — a zero that reads exactly like "rotation was never armed". + const knex = (driver as unknown as { knex: any }).knex; + const objects: Array<{ name: string; type: string }> = await knex + .raw(`SELECT name, type FROM sqlite_master`) + .then((r: any) => (Array.isArray(r) ? r : r?.rows ?? [])); + + expect(objects.find((o) => o.name === T)?.type).toBe('view'); + expect(objects.filter((o) => o.name.startsWith(`${T}__r`)).map((o) => o.name)).not.toHaveLength(0); + }); + + it('§2 rotateShards: the inline `indexes` + `tenancy` literals compile AND reach the shard', async () => { + const T = 'os16711_rotate'; + driver = new SqlDriver(SQLITE.config()); + + // The PUBLIC entry point of the rotation chain, with a fresh literal + // carrying both keys the chain's leaf (`ensureShardTable`) reads. Before + // #16711 this call did not compile: `rotateShards` declared neither key, + // and the two casts that read them sat three links down. + const state = await driver.rotateShards({ + ...bareObject(T), + tenancy: { enabled: false }, + indexes: [{ fields: ['v'], unique: true as const, name: `uniq_${T}_v` }], + lifecycle: ROTATION, + }); + + expect(state.shards.length).toBeGreaterThan(0); + + // The runtime half: the declared UNIQUE physically landed on the shard the + // chain created. Dropping `indexes` leaves it unsynced — silently. + const knex = (driver as unknown as { knex: any }).knex; + await knex(state.current).insert({ id: 'a', v: 'same' }); + await expect(knex(state.current).insert({ id: 'b', v: 'same' })).rejects.toThrow(); + }); + + it('§3 detectManagedDrift still reads `indexes` off its own declared parameter', async () => { + const T = 'os16711_drift'; + driver = new SqlDriver(SQLITE.config()); + await driver.initObjects([{ ...bareObject(T) }]); + + // `detectManagedDrift` declared `indexes?: any[]` all along and read it + // through an `as any` anyway — the residue of the same class. Removing the + // cast must not change what it sees. + const drift = await driver.detectManagedDrift([ + { ...bareObject(T), indexes: [{ fields: ['v'], unique: true as const, name: `uniq_${T}_v` }] }, + ]); + + expect(drift.some((d) => d.table === T)).toBe(true); + }); +}); + +/** + * ⭐ THE NEGATIVE CONTROL (#16711 验收口径 item 4). + * + * Widening is only a fix while the accept set still has a boundary. A parameter + * relaxed to `any`, or given an index signature, makes every assertion above + * green and every one of the lines below stop erroring — which is the one + * failure mode a green run cannot otherwise distinguish from a repair. + * + * Each `@ts-expect-error` is the assertion: `tsc` fails the file with TS2578 + * ("Unused '@ts-expect-error' directive") the moment the key starts being + * accepted. Compile-time only, deliberately never called. + */ +export async function refusesKeysThatAreNotDeclared(driver: SqlDriver): Promise { + const T = 'os16711_negative'; + + // @ts-expect-error TS2353 — `lifecycl` is a misspelling; nothing reads it. + await driver.initObjects([{ ...bareObject(T), lifecycl: ROTATION }]); + + // @ts-expect-error TS2353 — `indexs` is a misspelling; nothing reads it. + await driver.initObjects([{ ...bareObject(T), indexs: [] }]); + + // @ts-expect-error TS2353 — `tenancyy` is a misspelling; nothing reads it. + await driver.rotateShards({ ...bareObject(T), tenancyy: { enabled: false } }); + + // @ts-expect-error TS2353 — a key nobody ever declared anywhere on this class. + await driver.rotateShards({ ...bareObject(T), notAKeyAnyoneReads: 1 }); + + // @ts-expect-error TS2353 — the same, one method over, so the boundary is + // pinned on BOTH widened entry points and not just on the first. + driver.registerObjectMetadata([{ ...bareObject(T), alsoNotAKey: 1 }]); +} + +/** + * The narrowing axis, unchanged from #16570 and re-pinned here because the + * `lifecycle` widening touches the same literal: a variable-bound argument + * bypasses the excess-property check and is judged by ordinary assignability, + * so the declared TYPES still bind. Compile-time only. + */ +export async function pinsTheNarrowingAxis(driver: SqlDriver): Promise { + const asRecord = { ...bareObject('os16711_narrow'), indexes: { uniq_v: { fields: ['v'] } } }; + // @ts-expect-error TS2322 — a record is not `any[]`. + await driver.initObjects([asRecord]); +} diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index c135b8c915..37c3af1f3c 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -9374,7 +9374,7 @@ export class SqlDriver implements IDataDriver { * drops shards past the `shards × unit` window. */ async rotateShards( - objectDef: { name: string; fields?: Record; lifecycle?: any }, + objectDef: { name: string; fields?: Record; tenancy?: any; indexes?: any[]; lifecycle?: any }, nowMs: number = Date.now(), ): Promise<{ object: string; current: string; shards: string[]; dropped: string[] }> { this.assertSchemaMutable('rotateShards'); @@ -9391,7 +9391,7 @@ export class SqlDriver implements IDataDriver { protected async ensureRotation( tableName: string, - obj: { name: string; fields?: Record }, + obj: { name: string; fields?: Record; tenancy?: any; indexes?: any[] }, policy: { shards: number; unit: 'day' | 'week' | 'month' }, nowMs: number = Date.now(), ): Promise<{ object: string; current: string; shards: string[]; dropped: string[] }> { @@ -9544,8 +9544,21 @@ export class SqlDriver implements IDataDriver { } /** Create/column-sync one physical shard table (mirrors the managed-table - * branch of {@link initObjects}, scoped to a shard). */ - protected async ensureShardTable(shardName: string, obj: { fields?: Record; tenancy?: any }): Promise { + * branch of {@link initObjects}, scoped to a shard). + * + * #16711: `indexes` and `tenancy` are DECLARED here, on {@link ensureRotation} + * and on {@link rotateShards}, because this leaf reads both off the object the + * public entry point was handed — a shard carries the base table's declared + * indexes (#11374) and must scope a `unique: 'organization'` index the same + * way on every shard (ADR-0120 D1). Declaring them only here would leave the + * two links above still narrowing the same value, so a caller spelling + * `indexes` in a fresh literal to `rotateShards` would still be refused by a + * type while the driver read the key regardless. + */ + protected async ensureShardTable( + shardName: string, + obj: { fields?: Record; tenancy?: any; indexes?: any[] }, + ): Promise { const builtinColumns = new Set(['id', 'created_at', 'updated_at']); // [#12015] Both branches below drop a declared field named after a builtin // column — the create branch skips it explicitly, the column-sync branch @@ -9559,7 +9572,7 @@ export class SqlDriver implements IDataDriver { table: shardName, fields: obj.fields ?? {}, tenantField: this.resolveTenantField(shardName), - declaredIndexes: (obj as any).indexes, + declaredIndexes: obj.indexes, }); if (!exists) { await this.knex.schema.createTable(shardName, (table) => { @@ -9586,7 +9599,7 @@ export class SqlDriver implements IDataDriver { // Declared indexes per shard. Auto-derived names already embed the shard // name; explicit names get a shard prefix so they can't collide across // shards in the same database. - const declared = (obj as any).indexes; + const declared = obj.indexes; if (Array.isArray(declared) && declared.length > 0) { const colInfo = await this.knex(shardName).columnInfo(); const perShard = declared.map((idx: any) => ({ @@ -9945,8 +9958,19 @@ export class SqlDriver implements IDataDriver { // happened to bind first — a green that held for a reason unrelated to // correctness. `src/sql-driver-16570-init-objects-indexes-param.test.ts` // pins the fresh-literal form so it cannot silently go back. + // + // `lifecycle` was the third instance, and the one that made #16711 file the + // CLASS rather than a fourth single-key card: the loop below reads + // `obj.lifecycle?.storage` to decide whether a table is time-sharded, while + // the sibling `rotateShards` on this same class had always declared the key. + // Dropping it does not fail — it leaves the ADR-0057 rotation policy unarmed, + // silently, exactly as dropping `indexes` leaves a declared UNIQUE unsynced. + // `scripts/check-object-def-param-keys.mjs` now holds the whole class, + // including the half no in-file gate could see: a SUBCLASS in another + // published package overriding one of these methods with a narrower literal + // (`TursoDriver.initObjects` shadowed #4311's `tenancy` fix for five weeks). async initObjects( - objects: Array<{ name: string; fields?: Record; tenancy?: any; indexes?: any[] }>, + objects: Array<{ name: string; fields?: Record; tenancy?: any; indexes?: any[]; lifecycle?: any }>, ): Promise { // In-memory registration FIRST, and deliberately ahead of the DDL gate // below: being refused permission to alter a schema is not a reason to stay @@ -10004,7 +10028,7 @@ export class SqlDriver implements IDataDriver { // ADR-0057 P2: rotation-declared telemetry is physically time-sharded — // the Rotator owns its DDL (shard tables + a read view under the base // name); the plain create/alter path below would collide with the view. - const rotationPolicy = (obj as any).lifecycle?.storage; + const rotationPolicy = obj.lifecycle?.storage; if (rotationPolicy?.strategy === 'rotation' && this.supportsRotation) { this.tablesWithTimestamps.add(tableName); await this.ensureRotation(tableName, obj, rotationPolicy); @@ -11220,7 +11244,7 @@ export class SqlDriver implements IDataDriver { for (const o of objects) { tables.set(StorageNameMapping.resolveTableName(o), { fields: o.fields ?? {}, - indexes: (o as any).indexes, + indexes: o.indexes, }); } } else { diff --git a/packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-16711-inherited-object-def-keys.test.ts b/packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-16711-inherited-object-def-keys.test.ts new file mode 100644 index 0000000000..5df0b6b37a --- /dev/null +++ b/packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-16711-inherited-object-def-keys.test.ts @@ -0,0 +1,113 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #16711 — the INHERITING half of the subclass-shadowing class, asserted rather +// than assumed. +// +// `SqliteWasmDriver extends SqlDriver` and overrides NEITHER `initObjects` nor +// `registerObjectMetadata`, so its published surface re-declares no parameter +// type of its own: the door it exposes is `SqlDriver`'s, read through +// `@objectstack/driver-sql`'s built `.d.ts`. That is the OPPOSITE direction of +// the defect this card is about — `TursoDriver` overrides `initObjects` with a +// narrower literal and shadowed #4311's `tenancy` fix for five weeks — and the +// card's 验收口径 item 3 is explicit that this side must be MEASURED, not +// assumed: 「driver-sqlite-wasm(继承,应自动跟随 —— 断言它确实跟随了, +// ⛔ 不要假定)」. +// +// The assertion lives in this package's own tsc program (`tsconfig.json` +// selects `src/**/*`, tests included), so it is answered by the same `.d.ts` +// resolution a downstream consumer of `@objectstack/driver-sqlite-wasm` gets. +// +// Two ways this file goes red, both by design: +// - `@objectstack/driver-sql` narrows one of these parameters again — the +// inline literal below stops compiling here, in the package that would +// otherwise have gone on silently exposing the old shape; +// - a future override appears in `sqlite-wasm-driver.ts` that re-declares +// `initObjects` with a narrower literal — the same line goes red, naming +// the drift here rather than at a consumer, which is exactly the reading +// nobody had for `TursoDriver` between August and this card. +// +// The negative control at the bottom is what keeps that green from being free: +// a `SqlDriver` parameter relaxed to `any`, or given an index signature, would +// satisfy every line above while deleting the protection they are about. + +import { describe, it, expect, afterEach } from 'vitest'; +import { SqliteWasmDriver } from './index.js'; + +/** `any` defeats ordinary assignability checks; this is the standard detector. */ +type IsAny = 0 extends 1 & T ? true : false; + +type InitObjectsArg = Parameters[0]; +type InitObjectsElement = InitObjectsArg extends Array ? E : never; + +// 1. The inherited door is not masked. `any` would make every assertion below +// vacuous, so this is asserted before anything is read off the type. +const initObjectsArgIsAny: IsAny = false; +const initObjectsElementIsAny: IsAny = false; + +// 2. Every key SqlDriver declares is visible on the INHERITED element type. +// `Pick` fails to compile if the key is absent, so these four consts are the +// assertion; the `true`s are only how they reach a runtime expectation. +const inheritsTenancy: Pick extends object ? true : never = true; +const inheritsIndexes: Pick extends object ? true : never = true; +const inheritsLifecycle: Pick extends object ? true : never = true; +const inheritsFields: Pick extends object ? true : never = true; + +describe('SqliteWasmDriver inherits SqlDriver object-definition parameters (#16711)', () => { + let driver: SqliteWasmDriver | undefined; + afterEach(async () => { + await driver?.disconnect().catch(() => {}); + driver = undefined; + }); + + it('pins the inherited parameter shape at the type level', () => { + expect([initObjectsArgIsAny, initObjectsElementIsAny]).toEqual([false, false]); + expect([inheritsTenancy, inheritsIndexes, inheritsLifecycle, inheritsFields]).toEqual([true, true, true, true]); + }); + + it('accepts a FRESH object literal carrying the inherited keys, and reads them', async () => { + driver = new SqliteWasmDriver({ filename: ':memory:' }); + const T = 'os16711_wasm'; + + // Fresh literal in argument position — the only spelling that can go red on + // this defect, since TypeScript's excess-property check does not fire on a + // value bound to a variable first. + await driver.initObjects([ + { + name: T, + fields: { v: { type: 'text', maxLength: 64 }, organization_id: { type: 'text', maxLength: 64 } }, + tenancy: { enabled: false }, + indexes: [{ fields: ['v'], unique: true as const, name: `uniq_${T}_v` }], + }, + ]); + + // The runtime half: `tenancy: { enabled: false }` is READ, not merely + // admitted — the explicit opt-out beats the implicit `organization_id` + // heuristic, which is the reading that would silently flip if an author hit + // a TS2353 and dropped the key. + const tenantField = + (driver as unknown as { tenantFieldByTable: Record }).tenantFieldByTable[T] ?? null; + expect(tenantField).toBeNull(); + + // And `indexes` reached the physical schema. + const knex = (driver as unknown as { knex: any }).knex; + await knex(T).insert({ id: 'a', v: 'same' }); + await expect(knex(T).insert({ id: 'b', v: 'same' })).rejects.toThrow(); + }); +}); + +/** + * ⭐ THE NEGATIVE CONTROL (#16711 验收口径 item 4), on the INHERITED door. + * + * The `@ts-expect-error` IS the assertion: `tsc` fails this file with TS2578 + * the moment the base parameter starts accepting anything, which is what a + * relaxation to `any` or an index signature upstream would do — and this + * package would otherwise be the last place anyone looked. Compile-time only, + * deliberately never called. + */ +export async function refusesKeysThatAreNotDeclared(driver: SqliteWasmDriver): Promise { + // @ts-expect-error TS2353 — `tenancyy` is a misspelling; nothing reads it. + await driver.initObjects([{ name: 't', tenancyy: { enabled: false } }]); + + // @ts-expect-error TS2353 — a key nobody declares anywhere on either class. + await driver.initObjects([{ name: 't', notAKeyAnyoneReads: 1 }]); +} diff --git a/packages/drivers/driver-turso/src/turso-driver-16711-init-objects-param.test.ts b/packages/drivers/driver-turso/src/turso-driver-16711-init-objects-param.test.ts new file mode 100644 index 0000000000..f4930dd68b --- /dev/null +++ b/packages/drivers/driver-turso/src/turso-driver-16711-init-objects-param.test.ts @@ -0,0 +1,121 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#16711] `TursoDriver.initObjects` — the override that shadowed a base-class + * fix in a separately published package for five weeks. + * + * ## The defect this pins, which no gate scoped to `sql-driver.ts` could see + * + * `TursoDriver extends SqlDriver` and OVERRIDES `initObjects`. An `override` + * does **not** inherit the base's parameter type, so this class's own literal — + * `Array<{ name: string; fields?: Record }>` — is what every + * caller of `@objectstack/driver-turso` saw. Consequences, both measured: + * + * - #4311 declared `tenancy` on `SqlDriver.initObjects` in August. From + * outside this package that fix did not exist: a fresh literal carrying + * `tenancy` was still TS2353 here, for five weeks, and nothing was red. + * - #16570's `indexes` fix would have escaped by the identical route. + * + * ⭐ And the type face was the ONLY thing refusing them. The remote arm below + * forwards the whole object through as `schema` + * (`syncSchemasBatch(objects.map((obj) => ({ object: obj.name, schema: obj })))`), + * and `registerRemoteFieldMetadata` reads `tenancy` straight back off it — so + * the runtime carried both keys the entire time. That is this card's cleanest + * instance of "the type contradicts the runtime", and it lives in a different + * published package from the class it contradicts. + * + * ## Why the pin is an inline literal + * + * TypeScript's excess-property check fires on a **fresh object literal** and + * not on one bound to a variable first, so a variable-bound pin cannot go red + * on this defect at all. Every call below is inline in argument position, which + * makes this package's `typecheck` script the instrument. + * + * §3 is the negative control: a misspelling must still be refused. A "fix" that + * relaxed this parameter to `any` or gave it an index signature would turn §1 + * and §2 green while deleting the whole protection — and §3 is what says so. + */ + +import { describe, it, expect } from 'vitest'; +import { TursoDriver } from './turso-driver.js'; +import { makeLibsqlSqliteStub } from './libsql-sqlite-stub.testkit.js'; + +/** + * The base object, deliberately WITHOUT any key under test. `organization_id` + * is present as a field so the implicit-tenancy heuristic has something to find + * — which is what makes §2's opt-out reading non-vacuous. + */ +const bareObject = (name: string) => ({ + name, + fields: { + v: { type: 'text', maxLength: 64 }, + organization_id: { type: 'text', maxLength: 64 }, + }, +}); + +/** The tenant column this driver resolved for `object`, read off the registry. */ +const tenantFieldFor = (driver: TursoDriver, object: string): string | null => + (driver as unknown as { tenantFieldByTable: Record }).tenantFieldByTable[object] ?? null; + +async function remoteDriver() { + const driver = new TursoDriver({ url: 'libsql://probe.turso.io', client: makeLibsqlSqliteStub() as never }); + await driver.connect(); + expect(driver.transportMode).toBe('remote'); + return driver; +} + +describe('TursoDriver.initObjects declares every key SqlDriver.initObjects does (#16711)', () => { + it('§1 the inline literal carrying all four base keys compiles and is accepted', async () => { + const driver = await remoteDriver(); + const T = 'os16711_turso_all'; + + // Fresh literal in argument position. Before #16711 this did not compile: + // `tenancy`, `indexes` and `lifecycle` were all TS2353 against this + // override's own narrower literal, while the base declared the first two. + await driver.initObjects([ + { + ...bareObject(T), + tenancy: { enabled: true }, + indexes: [{ fields: ['v'], unique: true as const, name: `uniq_${T}_v` }], + lifecycle: { storage: { strategy: 'rotation' as const, shards: 2, unit: 'day' as const } }, + }, + ]); + + // The remote path registers read-coercion metadata for the object; that it + // did so is the evidence the call reached the driver rather than merely + // typechecking. + expect(tenantFieldFor(driver, T)).toBe('organization_id'); + }); + + it('§2 `tenancy` is READ, not merely admitted — the opt-out changes the tenant column', async () => { + const driver = await remoteDriver(); + const T = 'os16711_turso_optout'; + + // ⭐ The ruling's own example of why this key is not decoration: + // `tenancy.enabled: false` is what decides whether a UNIQUE partitions + // globally or per organization. An author who hit the old TS2353 and + // DROPPED the key would silently get the other answer. + await driver.initObjects([{ ...bareObject(T), tenancy: { enabled: false } }]); + + expect(tenantFieldFor(driver, T)).toBeNull(); + }); +}); + +/** + * ⭐ THE NEGATIVE CONTROL (#16711 验收口径 item 4). Each `@ts-expect-error` IS + * the assertion: `tsc` fails the file with TS2578 the moment the key starts + * being accepted, which is precisely what a relaxation to `any` or an index + * signature would do. Compile-time only, deliberately never called. + */ +export async function refusesKeysThatAreNotDeclared(driver: TursoDriver): Promise { + const T = 'os16711_turso_negative'; + + // @ts-expect-error TS2353 — `tenancyy` is a misspelling; nothing reads it. + await driver.initObjects([{ ...bareObject(T), tenancyy: { enabled: false } }]); + + // @ts-expect-error TS2353 — `indexs` is a misspelling; nothing reads it. + await driver.initObjects([{ ...bareObject(T), indexs: [] }]); + + // @ts-expect-error TS2353 — a key nobody declares anywhere on either class. + await driver.initObjects([{ ...bareObject(T), notAKeyAnyoneReads: 1 }]); +} diff --git a/packages/drivers/driver-turso/src/turso-driver.ts b/packages/drivers/driver-turso/src/turso-driver.ts index dd5b42f3e7..985c541795 100644 --- a/packages/drivers/driver-turso/src/turso-driver.ts +++ b/packages/drivers/driver-turso/src/turso-driver.ts @@ -1412,10 +1412,10 @@ export class TursoDriver extends SqlDriver { * already succeeded, so the table exists with its `id` primary key whether or * not the best-effort coercion registration below does. */ - private registerRemoteFieldMetadata(obj: { name: string; fields?: Record }): void { + private registerRemoteFieldMetadata(obj: { name: string; fields?: Record; tenancy?: any }): void { this.remoteManagedObjects.add(obj.name); try { - this.registerExternalObject({ name: obj.name, fields: obj.fields, tenancy: (obj as any).tenancy }); + this.registerExternalObject({ name: obj.name, fields: obj.fields, tenancy: obj.tenancy }); } catch { /* metadata registration is best-effort; never block schema sync on it */ } @@ -1639,9 +1639,26 @@ export class TursoDriver extends SqlDriver { * which uses `@libsql/client.batch()` against the Turso endpoint directly. * * In local / replica modes the existing Knex-based path remains in effect. + * + * ⛔ #16711 — this parameter type must declare every key `SqlDriver.initObjects` + * declares, and `scripts/check-object-def-param-keys.mjs` fails the build if it + * stops doing so. An `override` does NOT inherit the base's parameter type, so + * this literal is what every caller of `@objectstack/driver-turso` sees: while + * it read `{ name; fields? }`, #4311's `tenancy` fix sat on the base for five + * weeks and was invisible from outside `@objectstack/driver-sql`, and #16570's + * `indexes` fix would have escaped the same way. The escape is silent because + * TypeScript's excess-property check fires on a FRESH object literal only — and + * the remote arm below forwards the WHOLE object as `schema`, so the runtime + * carried both keys the whole time and only the type face refused them. */ override async initObjects( - objects: Array<{ name: string; fields?: Record }>, + objects: Array<{ + name: string; + fields?: Record; + tenancy?: any; + indexes?: any[]; + lifecycle?: any; + }>, ): Promise { if (this.isRemote) { if (objects.length === 0) return; diff --git a/scripts/check-object-def-param-keys.mjs b/scripts/check-object-def-param-keys.mjs new file mode 100644 index 0000000000..6281d26096 --- /dev/null +++ b/scripts/check-object-def-param-keys.mjs @@ -0,0 +1,783 @@ +#!/usr/bin/env node +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * check-object-def-param-keys (#16711) -- an object-definition parameter must + * DECLARE every key that is read off it, including the keys its base class + * declared. + * + * node scripts/check-object-def-param-keys.mjs + * node scripts/check-object-def-param-keys.mjs --list + * node scripts/check-object-def-param-keys.mjs --self-test + * + * ## The measured failure, twice, in two packages, over five weeks + * + * `SqlDriver` takes object definitions as inline object-literal parameter types + * (`{ name: string; fields?: Record; ... }`) and then reads keys + * off them that the literal does not list, through `(obj as any).`. Three + * instances were carded one at a time before anyone called it a class: + * + * #4311 `tenancy` declared on `SqlDriver.initObjects` in August + * #16570 `indexes` declared on the same three entry points in September + * #16711 `lifecycle` the third, and the card that stopped counting them + * + * The failure is silent by construction. TypeScript's excess-property check + * fires on a FRESH object literal and not on one bound to a variable first, so + * the same object is accepted or refused depending only on where it is spelled: + * + * await driver.initObjects([{ ...bare, lifecycle: { storage: … } }]); // TS2353 + * const hoisted = { ...bare, lifecycle: { storage: … } }; + * await driver.initObjects([hoisted]); // accepted + * + * The loud outcome -- a compile error on a CORRECT call -- is the good one. The + * bad one is an author, or an AI reading the signature, concluding the key is + * not accepted and DROPPING it: a declared UNIQUE that is never synced, an + * ADR-0057 rotation policy that is never armed, and nothing anywhere says so. + * + * ## ⭐ Why this gate is NOT scoped to one file, which is the whole point + * + * The obvious gate -- sweep `sql-driver.ts` for `(obj as any).` against + * the parameter types in the same file -- would have caught NEITHER historical + * escape, and #16711's triage ruling says so in one sentence: + * + * ⛔ 闸门若只盯 `sql-driver.ts`,#4311 与本次都拦不住 + * + * `TursoDriver extends SqlDriver` and OVERRIDES `initObjects`. An override does + * not inherit the base's parameter type, so its own, narrower literal is what + * every caller of `@objectstack/driver-turso` sees. #4311's `tenancy` fix + * landed on the base in August and was invisible from outside that package for + * five weeks; #16570's `indexes` fix would have escaped identically. Meanwhile + * the override's remote arm sends the whole object through as `schema`, so the + * RUNTIME carries both keys and only the type face refuses them. + * + * So the question this gate answers is not "which keys does `SqlDriver` read + * but not declare". It is the one the ruling reframed it into: + * + * 「一个子类在另一个已发布的包里静默遮蔽了基类的声明, + * 使基类的修复从外面看不见」 + * + * ## The three arms + * + * **A -- OVERRIDE NARROWING.** For every class in the corpus that extends + * another class in the corpus, every method present on both: each parameter + * position whose base type is an inline object literal (or an array of one) + * must declare every key the base declares. This arm crosses package + * boundaries by construction -- the corpus is the workspace, not a directory. + * + * **B -- UNDECLARED CAST READ.** Inside a method, `(x as any).` where `x` + * is a parameter annotated with an inline object literal -- or a `for…of` + * binding over a parameter annotated with an array of one -- and `` is not + * in that literal. This is the original #16711 census instrument, generalised + * off `sql-driver.ts` and onto every driver source in the tree. + * + * **C -- THE ESCAPE HATCHES.** Both arms above are made vacuous by two edits + * that look like fixes: giving the parameter an index signature + * (`[key: string]: unknown`), or replacing a base's object literal with an + * opaque annotation in the override. Either makes every key "declared" and + * deletes the whole layer of protection this class is about. #16711's 验收口径 + * item 4 names exactly this shape as the failure mode its negative control + * exists for, so the gate refuses it directly rather than trusting a reviewer + * to notice. Measured 0 on the tree this landed against, so the ledger below is + * empty and adding to it is a maintainer's call. + * + * ## ⚠️ Why the corpus carries a POSITIVE CONTROL and refuses without it + * + * This gate's own development reproduced, on the first run, the exact trap + * #16711's PM comment recorded on the file it scans: a `git ls-files` pathspec + * of `packages`, a slash, a star, a slash, `src`, a slash, a double star, a + * slash and `*.ts` matched only files at least one directory BELOW + * `src/`, so `sql-driver.ts` and `turso-driver.ts` were both outside the + * corpus. The gate printed `0 violations` and exited 0. Nothing about that + * reading was distinguishable from a clean tree. + * + * A zero whose control does not fire has measured NOTHING. So the corpus + * enumeration is checked against {@link REQUIRED_CORPUS_FILES} -- files that + * must be in it for any verdict to mean anything -- and a run that cannot see + * them exits {@link EXIT_CORPUS_UNVERIFIED} rather than green. That is the same + * distinction `scripts/ts-parse.mjs` draws with `EXIT_UNPARSEABLE`: "nothing to + * report" and "I could not read it" are different answers and must not share an + * exit code. + * + * ## Why an AST and not a regex + * + * The signature this class hides behind WRAPS ACROSS LINES. #16711's PM comment + * records a single-line grep for `indexes|tenancy` over the Turso override + * returning nothing -- and its control returning nothing too, because + * `override async initObjects(` and its parameter are on different lines. That + * was a silence, not a negative, and it is the second time that trap fired on + * that seat in one night. A parameter list is a tree, so this gate reads it as + * one, through the repo's single sanctioned parser entry point + * (`scripts/ts-parse.mjs#parseSourceFile`), which refuses a source it could not + * read instead of scoring it clean. + * + * ## What it does NOT claim + * + * It compares DECLARED key sets, not assignability. It cannot tell you that a + * declared `indexes?: any[]` is the right type for what the body does with it, + * and it does not look at named type aliases or interfaces -- an object + * definition passed as `ObjectMeta` is out of scan scope in both arms, because + * a named type has one declaration site and does not have this class's failure + * mode (a sibling method silently disagreeing about the same input). Arm C is + * what keeps "make it a named opaque type" from being a route to green. + * + * The live sites this gate was built against, as symbol anchors so they do not + * rot: `packages/drivers/driver-sql/src/sql-driver.ts#ensureShardTable`, + * (`#initObjects`, `#registerObjectMetadata`, `#registerManagedObjectMetadata`, + * `#detectManagedDrift`, `#rotateShards`, `#ensureRotation`) and + * `packages/drivers/driver-turso/src/turso-driver.ts#TursoDriver` — whose + * `initObjects` override and `registerRemoteFieldMetadata` helper are the two + * members this gate reads there. ⚠️ Neither is spelled as a symbol anchor, + * because neither has a resolvable declaration site under the shared resolver's + * rule: `override async initObjects(` puts the name mid-line, which is the SAME + * wrapped-signature shape that made a single-line grep for it return a silence + * on this very file. Anchoring the class instead keeps the citation checked. + */ + +import { execFileSync } from 'node:child_process'; +import { readFileSync } from 'node:fs'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { gitFreeEnv } from './git-env.mjs'; +import { requireDefaultExport } from './import-prerequisite.mjs'; +import { isEntrypoint } from './invoked-as.mjs'; +import { parseSourceFile } from './ts-parse.mjs'; + +const ts = await requireDefaultExport('typescript', () => import('typescript'), import.meta.url); + +const ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..'); + +/** + * The exit status of "I could not read the tree I am supposed to judge". + * + * Distinct from 1 ("found violations") and from 0 ("nothing to report") for the + * reason the header's positive-control section gives: those are three different + * answers and a shared code makes two of them unreachable. + */ +export const EXIT_CORPUS_UNVERIFIED = 3; + +/** + * Files whose presence in the corpus is the gate's positive control. + * + * ⛔ These are not "important files"; they are the files whose ABSENCE has + * already been observed to turn this gate into a green no-op. Both carry a live + * class-member declaration this gate must be able to see, so a corpus that + * misses either one cannot have judged the class at all. + */ +export const REQUIRED_CORPUS_FILES = [ + 'packages/drivers/driver-sql/src/sql-driver.ts', + 'packages/drivers/driver-turso/src/turso-driver.ts', + 'packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-driver.ts', +]; + +/** + * Arm-C exemptions, keyed `::.#`. + * + * ⛔ SHRINK-ONLY, and a maintainer's call. An entry here says "this parameter + * may erase the base's declared shape", which switches arms A and B off for it. + * Measured empty on the tree this gate landed against; an author whose override + * needs a different type widens the BASE instead, which keeps both arms live. + */ +export const ARM_C_EXEMPT = Object.freeze({}); + +/** + * The corpus: every tracked TypeScript source under a package's `src/`, minus + * test and spec files. + * + * ⚠️ Enumerated with a plain `git ls-files packages` and filtered in JS on + * purpose. The star-slash-double-star pathspec spelling reads as though it says + * this, and does not: git's `**` requires at least one intervening directory, + * so every file sitting directly in a `src/` -- which is where both driver + * entry points live -- falls out of it silently. The filter below is the same + * predicate written where it can be read. + */ +export function corpusFiles(root = ROOT) { + const out = execFileSync('git', ['-C', root, 'ls-files', 'packages'], { + encoding: 'utf8', + maxBuffer: 64 * 1024 * 1024, + env: gitFreeEnv(), + }); + return out + .split('\n') + .filter(Boolean) + .filter((f) => f.endsWith('.ts') && f.includes('/src/') && !/\.(test|spec)\.ts$/.test(f)); +} + +/** + * The declared key set of an inline object-literal type, plus whether it + * carries an index signature. + */ +function literalKeys(node) { + const keys = []; + let indexSignature = false; + for (const member of node.members) { + if (ts.isIndexSignatureDeclaration(member)) { + indexSignature = true; + continue; + } + if (!ts.isPropertySignature(member) || !member.name) continue; + if (ts.isIdentifier(member.name) || ts.isStringLiteral(member.name)) keys.push(member.name.text); + } + return { keys, indexSignature }; +} + +/** + * Classify a parameter's type annotation. + * + * `object` an inline object literal -- judged + * `array` an array of an inline object literal -- judged, element-wise + * `opaque` anything else (named type, union, `any`, …) -- not judged + * `none` no annotation at all -- not judged + * + * Only the first two carry a decidable key set, which is the whole reason the + * other two are reported as a kind rather than merged into "unknown": arm C + * reads the difference between them and a base that HAD one. + */ +export function classifyParamType(typeNode) { + if (!typeNode) return { kind: 'none' }; + if (ts.isTypeLiteralNode(typeNode)) return { kind: 'object', ...literalKeys(typeNode) }; + if (ts.isArrayTypeNode(typeNode) && ts.isTypeLiteralNode(typeNode.elementType)) { + return { kind: 'array', ...literalKeys(typeNode.elementType) }; + } + if ( + ts.isTypeReferenceNode(typeNode) + && ts.isIdentifier(typeNode.typeName) + && typeNode.typeName.text === 'Array' + && typeNode.typeArguments?.length === 1 + && ts.isTypeLiteralNode(typeNode.typeArguments[0]) + ) { + return { kind: 'array', ...literalKeys(typeNode.typeArguments[0]) }; + } + return { kind: 'opaque', text: typeNode.getText() }; +} + +/** Strip parentheses so `((x as any)).k` reads the same as `(x as any).k`. */ +function unwrap(expr) { + let e = expr; + while (ts.isParenthesizedExpression(e)) e = e.expression; + return e; +} + +/** + * One source file's contribution: the classes it declares (with each method's + * parameter shapes) and every arm-B cast read inside it. + */ +export function analyzeSource(fileName, text) { + const sourceFile = parseSourceFile(fileName, text, ts.ScriptKind.TS); + const lineOf = (node) => sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)).line + 1; + + const classes = []; + const castReads = []; + + const collectCastReads = (fnNode, ownerLabel) => { + /** identifier -> the object shape it is known to carry */ + const env = new Map(); + for (const param of fnNode.parameters) { + if (!ts.isIdentifier(param.name)) continue; + const shape = classifyParamType(param.type); + if (shape.kind === 'object' || shape.kind === 'array') { + env.set(param.name.text, { ...shape, origin: 'a parameter' }); + } + } + + const walk = (node) => { + // `for (const obj of objects)` where `objects` is an array-shaped + // parameter: the binding carries the element shape. This is not a nicety + // -- #16711's `lifecycle` read is spelled exactly this way, and a gate + // that only followed parameters directly would have missed it. + if ( + ts.isForOfStatement(node) + && ts.isVariableDeclarationList(node.initializer) + && node.initializer.declarations.length === 1 + && ts.isIdentifier(node.expression) + ) { + const decl = node.initializer.declarations[0]; + const source = env.get(node.expression.text); + if (ts.isIdentifier(decl.name) && source?.kind === 'array') { + env.set(decl.name.text, { + kind: 'object', + keys: source.keys, + indexSignature: source.indexSignature, + origin: `an element of \`${node.expression.text}\``, + }); + } + } + + if (ts.isPropertyAccessExpression(node) || ts.isElementAccessExpression(node)) { + const inner = unwrap(node.expression); + if ( + ts.isAsExpression(inner) + && inner.type.kind === ts.SyntaxKind.AnyKeyword + && ts.isIdentifier(inner.expression) + ) { + const shape = env.get(inner.expression.text); + const key = ts.isPropertyAccessExpression(node) + ? node.name.text + : (ts.isStringLiteral(node.argumentExpression) ? node.argumentExpression.text : null); + if (shape && key && !shape.indexSignature && !shape.keys.includes(key)) { + castReads.push({ + file: fileName, + line: lineOf(node), + owner: ownerLabel, + identifier: inner.expression.text, + key, + origin: shape.origin, + declared: shape.keys, + }); + } + } + } + + ts.forEachChild(node, walk); + }; + + if (fnNode.body) walk(fnNode.body); + }; + + const visit = (node) => { + if ((ts.isClassDeclaration(node) || ts.isClassExpression(node)) && node.name) { + const extendsClause = node.heritageClauses?.find((h) => h.token === ts.SyntaxKind.ExtendsKeyword); + const baseExpr = extendsClause?.types?.[0]?.expression; + const methods = new Map(); + for (const member of node.members) { + if (!ts.isMethodDeclaration(member) || !member.name || !ts.isIdentifier(member.name)) continue; + methods.set(member.name.text, { + line: lineOf(member), + params: member.parameters.map((p) => classifyParamType(p.type)), + }); + } + classes.push({ + name: node.name.text, + base: baseExpr && ts.isIdentifier(baseExpr) ? baseExpr.text : null, + file: fileName, + line: lineOf(node), + methods, + }); + } + + if (ts.isMethodDeclaration(node) && node.name && ts.isIdentifier(node.name)) { + collectCastReads(node, node.name.text); + } else if (ts.isFunctionDeclaration(node) && node.name) { + collectCastReads(node, node.name.text); + } + + ts.forEachChild(node, visit); + }; + + visit(sourceFile); + return { classes, castReads }; +} + +/** + * Arms A and C, over the whole class index. + * + * A base class is resolved BY NAME. Two classes sharing a name is not resolved + * by guessing: the pair is reported as `ambiguous` and judged by nothing, which + * is visible in `--list` instead of silently skipped. + */ +export function findOverrideFindings(classes) { + const byName = new Map(); + for (const c of classes) { + if (!byName.has(c.name)) byName.set(c.name, []); + byName.get(c.name).push(c); + } + + const narrowing = []; + const erasure = []; + const ambiguous = []; + let comparedPairs = 0; + + for (const derived of classes) { + if (!derived.base) continue; + const candidates = byName.get(derived.base); + if (!candidates || candidates.length === 0) continue; + if (candidates.length > 1) { + ambiguous.push({ derived: derived.name, file: derived.file, base: derived.base, count: candidates.length }); + continue; + } + const base = candidates[0]; + if (base === derived) continue; + + for (const [methodName, method] of derived.methods) { + const baseMethod = base.methods.get(methodName); + if (!baseMethod) continue; + const arity = Math.min(method.params.length, baseMethod.params.length); + for (let i = 0; i < arity; i++) { + const basePar = baseMethod.params[i]; + const derPar = method.params[i]; + if (basePar.kind !== 'object' && basePar.kind !== 'array') continue; + comparedPairs += 1; + + const exemptKey = `${derived.file}::${derived.name}.${methodName}#${i}`; + if (Object.prototype.hasOwnProperty.call(ARM_C_EXEMPT, exemptKey)) continue; + + if (derPar.kind === 'opaque' || derPar.kind === 'none') { + erasure.push({ + kind: 'erased', + file: derived.file, + line: method.line, + derived: derived.name, + base: base.name, + baseFile: base.file, + method: methodName, + param: i, + detail: derPar.kind === 'none' ? '(no annotation)' : derPar.text, + exemptKey, + }); + continue; + } + if (derPar.indexSignature || basePar.indexSignature) { + erasure.push({ + kind: 'index-signature', + file: derPar.indexSignature ? derived.file : base.file, + line: derPar.indexSignature ? method.line : baseMethod.line, + derived: derived.name, + base: base.name, + baseFile: base.file, + method: methodName, + param: i, + detail: 'an index signature makes every key "declared"', + exemptKey, + }); + continue; + } + + const missing = basePar.keys.filter((k) => !derPar.keys.includes(k)); + if (missing.length > 0) { + narrowing.push({ + file: derived.file, + line: method.line, + derived: derived.name, + base: base.name, + baseFile: base.file, + baseLine: baseMethod.line, + method: methodName, + param: i, + missing, + baseKeys: basePar.keys, + derivedKeys: derPar.keys, + }); + } + } + } + } + + return { narrowing, erasure, ambiguous, comparedPairs }; +} + +/** Read the whole corpus and produce every finding, in one pass. */ +export function sweep(root = ROOT) { + const files = corpusFiles(root); + const missingControls = REQUIRED_CORPUS_FILES.filter((f) => !files.includes(f)); + + const classes = []; + const castReads = []; + if (missingControls.length === 0) { + for (const rel of files) { + const analyzed = analyzeSource(rel, readFileSync(resolve(root, rel), 'utf8')); + classes.push(...analyzed.classes); + castReads.push(...analyzed.castReads); + } + } + + return { files, missingControls, classes, castReads, ...findOverrideFindings(classes) }; +} + +// ───────────────────────────── self-test ───────────────────────────── + +const SELF_TEST_VERDICT = 'check-object-def-param-keys self-test: OK'; + +/** + * The `TursoDriver.initObjects` signature EXACTLY as it stood on `main` before + * #16711 -- the gate's permanent firing control. + * + * ⭐ #16711's 验收口径 item 2: 「闸门自己要有发火对照…⛔ 只在修复后跑一次绿的 + * 闸门,与没有闸门无法区分」. Once the tree is fixed, the production sweep is + * green forever, and a green sweep is the one reading that cannot tell a + * working gate from a broken one. This fixture is the reading that can: it is + * the real historical shape, it must go RED, and it stays in the self-test long + * after the source it was copied from stopped looking like this. + * + * ⛔ Do not "update" it to today's signature. It is a dated record of a defect, + * not a mirror of the file. + */ +const TURSO_OVERRIDE_BEFORE_16711 = ` +class SqlDriver { + async initObjects( + objects: Array<{ name: string; fields?: Record; tenancy?: any; indexes?: any[] }>, + ): Promise {} +} +class TursoDriver extends SqlDriver { + override async initObjects( + objects: Array<{ name: string; fields?: Record }>, + ): Promise {} +} +`; + +function analyzeSnippet(source) { + const { classes, castReads } = analyzeSource('fixture.ts', source); + return { ...findOverrideFindings(classes), castReads }; +} + +export function selfTest() { + const failures = []; + const t = (name, ok) => { if (!ok) failures.push(name); }; + + // ── Arm A: the firing control, and both directions around it ── + const before = analyzeSnippet(TURSO_OVERRIDE_BEFORE_16711); + t( + 'FIRING CONTROL: the pre-#16711 TursoDriver override is RED for both keys', + before.narrowing.length === 1 + && before.narrowing[0].method === 'initObjects' + && before.narrowing[0].missing.join(',') === 'tenancy,indexes', + ); + t( + 'FIRING CONTROL: it is red because of the SUBCLASS, not the base', + before.narrowing[0]?.derived === 'TursoDriver' && before.narrowing[0]?.base === 'SqlDriver', + ); + t('the widened override is green', analyzeSnippet(` +class SqlDriver { + async initObjects(objects: Array<{ name: string; fields?: Record; tenancy?: any; indexes?: any[] }>): Promise {} +} +class TursoDriver extends SqlDriver { + override async initObjects(objects: Array<{ name: string; fields?: Record; tenancy?: any; indexes?: any[] }>): Promise {} +} +`).narrowing.length === 0); + t('an override declaring MORE than the base is green', analyzeSnippet(` +class A { m(o: { a?: 1 }): void {} } +class B extends A { override m(o: { a?: 1; b?: 2 }): void {} } +`).narrowing.length === 0); + t('a method the base does not have is not compared', analyzeSnippet(` +class A { m(o: { a?: 1 }): void {} } +class B extends A { other(o: {}): void {} } +`).narrowing.length === 0); + t('a class with no resolvable base is not compared', analyzeSnippet(` +class B extends Unknown { m(o: {}): void {} } +`).narrowing.length === 0); + t('a duplicated base name is reported ambiguous, never guessed', analyzeSnippet(` +class A { m(o: { a?: 1 }): void {} } +class A { m(o: { a?: 1 }): void {} } +class B extends A { override m(o: {}): void {} } +`).ambiguous.length === 1); + + // ⭐ The wrap trap, pinned. #16711's PM comment recorded a single-line grep + // for these two keys over this exact shape returning nothing -- AND its + // control returning nothing -- because the parameter is on its own line. + const wrapped = analyzeSnippet(` +class A { + m( + objects: Array<{ + name: string; + tenancy?: any; + }>, + ): void {} +} +class B extends A { + override m( + objects: Array<{ + name: string; + }>, + ): void {} +} +`); + t('a signature wrapped across lines is still read (the recorded grep trap)', + wrapped.narrowing.length === 1 && wrapped.narrowing[0].missing.join(',') === 'tenancy'); + + // ── Arm B: the undeclared cast read ── + const armB = analyzeSnippet(` +class A { + m(obj: { fields?: Record; tenancy?: any }): void { + const x = (obj as any).indexes; + } +} +`); + t('a cast read of an undeclared key is RED', + armB.castReads.length === 1 && armB.castReads[0].key === 'indexes'); + t('a cast read of a DECLARED key is green', analyzeSnippet(` +class A { + m(obj: { fields?: Record; indexes?: any[] }): void { + const x = (obj as any).indexes; + } +} +`).castReads.length === 0); + + // ⭐ The `for…of` binding: #16711's `lifecycle` read is spelled this way, so a + // gate that only followed parameters directly would have scored it clean. + const loop = analyzeSnippet(` +class A { + m(objects: Array<{ name: string; fields?: Record }>): void { + for (const obj of objects) { + const p = (obj as any).lifecycle?.storage; + } + } +} +`); + t('a cast read on a for…of binding over an array parameter is RED', + loop.castReads.length === 1 && loop.castReads[0].key === 'lifecycle'); + t('optional-chained and element-access spellings are read too', analyzeSnippet(` +class A { + m(obj: { a?: 1 }): void { + const x = (obj as any)?.zzz; + const y = (obj as any)['yyy']; + } +} +`).castReads.length === 2); + t('a cast on a NON-parameter identifier is out of scan scope', analyzeSnippet(` +class A { + m(): void { + const local: any = {}; + const x = (local as any).whatever; + } +} +`).castReads.length === 0); + t('a parameter with an opaque named type is out of scan scope', analyzeSnippet(` +class A { + m(query: DriverQuery): void { + const x = (query as any).groupBy; + } +} +`).castReads.length === 0); + + // ── Arm C: the two edits that would make arms A and B vacuous ── + const erased = analyzeSnippet(` +class A { m(o: { a?: 1; b?: 2 }): void {} } +class B extends A { override m(o: any): void {} } +`); + t('replacing the base literal with an opaque type is RED (not silently skipped)', + erased.erasure.length === 1 && erased.erasure[0].kind === 'erased'); + const idx = analyzeSnippet(` +class A { m(o: { a?: 1; b?: 2 }): void {} } +class B extends A { override m(o: { a?: 1; b?: 2; [k: string]: unknown }): void {} } +`); + t('an index signature on the override is RED', idx.erasure.length === 1 && idx.erasure[0].kind === 'index-signature'); + t('an index signature makes arm B silent, so arm C has to hold that line', analyzeSnippet(` +class A { + m(obj: { a?: 1; [k: string]: unknown }): void { + const x = (obj as any).anything; + } +} +`).castReads.length === 0); + + // ── The corpus predicate itself, which is where this gate's own bug was ── + const files = corpusFiles(); + for (const control of REQUIRED_CORPUS_FILES) { + t(`POSITIVE CONTROL: the corpus contains ${control}`, files.includes(control)); + } + t('the corpus excludes test files', !files.some((f) => /\.(test|spec)\.ts$/.test(f))); + t('the corpus is only package sources', files.every((f) => f.startsWith('packages/') && f.includes('/src/'))); + + if (failures.length > 0) { + console.error(`\n✗ check-object-def-param-keys self-test: ${failures.length} case(s) failed\n`); + for (const f of failures) console.error(` - ${f}`); + console.error(''); + process.exit(1); + } + console.log(`${SELF_TEST_VERDICT} (${files.length} corpus file(s), every control fired)`); + return SELF_TEST_VERDICT; +} + +// ───────────────────────────── main ───────────────────────────── + +function report(result) { + const { narrowing, erasure, castReads } = result; + const total = narrowing.length + erasure.length + castReads.length; + + if (total === 0) { + console.log( + `check:object-def-param-keys: OK — ${result.files.length} source file(s), ` + + `${result.classes.length} class(es), ${result.comparedPairs} override parameter position(s) compared.`, + ); + return 0; + } + + console.error(`check:object-def-param-keys: ${total} problem(s)\n`); + + if (narrowing.length > 0) { + console.error(' ── A · an override declares FEWER keys than the base it shadows ──'); + for (const v of narrowing) { + console.error(` ${v.file}:${v.line} ${v.derived} extends ${v.base} — ${v.method}(param ${v.param})`); + console.error(` drops: ${v.missing.join(', ')}`); + console.error(` base declares {${v.baseKeys.join('; ')}} at ${v.baseFile}:${v.baseLine}`); + } + console.error(''); + } + if (erasure.length > 0) { + console.error(' ── C · the base\'s declared shape is erased rather than widened ──'); + for (const v of erasure) { + console.error(` ${v.file}:${v.line} ${v.derived} extends ${v.base} — ${v.method}(param ${v.param}) [${v.kind}]`); + console.error(` ${v.detail}`); + } + console.error(''); + } + if (castReads.length > 0) { + console.error(' ── B · a key is READ off a parameter its own type does not declare ──'); + for (const v of castReads) { + console.error(` ${v.file}:${v.line} ${v.owner}: (${v.identifier} as any).${v.key}`); + console.error(` ${v.identifier} is ${v.origin} declared {${v.declared.join('; ')}}`); + } + console.error(''); + } + + console.error(`An object-definition parameter is a CONTRACT with the people who call it. +TypeScript's excess-property check fires on a fresh object literal and not on +one bound to a variable first, so a key the type omits is refused at some call +sites and accepted at others — and the author who hits the refusal drops the +key, which is silent: an unsynced UNIQUE, an unarmed rotation policy. + +Fix it by DECLARING the key on the parameter type — on the override AND on the +base, so a fix to one is visible from the other — and deleting the \`as any\`. +⛔ Not by loosening the parameter to \`any\` or adding an index signature: that +turns every line above green while deleting the protection they are about (arm +C above refuses exactly that, and #16711 item 4 is the negative control for it).`); + return 1; +} + +function main(argv) { + if (argv.includes('--self-test')) { + selfTest(); + return; + } + + if (selfTest() !== SELF_TEST_VERDICT) { + console.error( + '\n✗ check-object-def-param-keys: the self-test returned without reaching its verdict,\n' + + 'so the sweep below would be running on an unverified instrument.\n', + ); + process.exit(1); + } + + const result = sweep(); + + if (result.missingControls.length > 0) { + console.error(`\n✗ check-object-def-param-keys — REFUSING to report on a corpus that lost its controls.\n`); + console.error(` ${result.files.length} file(s) enumerated, but these are not among them:\n`); + for (const f of result.missingControls) console.error(` ${f}`); + console.error(` +Every one of those carries a declaration this gate exists to read, so a verdict +without them is not a clean tree — it is a gate that scanned the wrong set. This +is the failure this gate was written with: a \`git ls-files\` pathspec whose +\`**\` silently excluded every file sitting directly in a \`src/\`, which printed +"0 violations" and exited 0. + +If a control file was legitimately moved or renamed, update REQUIRED_CORPUS_FILES +in scripts/check-object-def-param-keys.mjs in the same commit.`); + process.exit(EXIT_CORPUS_UNVERIFIED); + } + + if (argv.includes('--list')) { + console.log(`corpus: ${result.files.length} file(s), ${result.classes.length} class(es)`); + console.log(`override parameter positions compared: ${result.comparedPairs}`); + for (const c of result.classes.filter((c) => c.base)) { + console.log(` ${c.file}:${c.line} ${c.name} extends ${c.base}`); + } + if (result.ambiguous.length > 0) { + console.log(`\nambiguous base names (judged by nothing):`); + for (const a of result.ambiguous) console.log(` ${a.file} ${a.derived} extends ${a.base} (${a.count} declarations)`); + } + } + + const status = report(result); + if (status !== 0) process.exit(status); +} + +if (isEntrypoint(import.meta.url)) main(process.argv.slice(2));