diff --git a/.changeset/stranded-orphan-inventory-createdat-dialect.md b/.changeset/stranded-orphan-inventory-createdat-dialect.md new file mode 100644 index 0000000000..6a88d9968a --- /dev/null +++ b/.changeset/stranded-orphan-inventory-createdat-dialect.md @@ -0,0 +1,32 @@ +--- +"@objectstack/service-storage": patch +--- + +fix(service-storage): report `createdAt` on every stranded-orphan sample, not only on SQLite (#13996) + +`inventoryStrandedFileOrphans` projects `created_at` out of the `sys_file` read +door and then tested it with `typeof row.created_at === 'string'`. `created_at` +is a BUILTIN audit column — it is not in `datetimeFields`, and +`SqlDriver#formatOutput` repairs the audit columns only inside its +`if (this.isSqlite)` arm — so that door hands the value back as canonical ISO-Z +text on SQLite and as a JS `Date` on Postgres and MySQL, the production default +drivers (pinned per dialect in driver-sql's +`sql-driver-13567-audit-stamp-materialisation.test.ts`). + +The guard was therefore `false` for **every** row on both live dialects: a field +explicitly asked for from the driver was silently discarded, and every sample in +an operator's stranded-orphan report carried `createdAt: undefined` there while +looking correct on the SQLite the suite runs on. + +The consumer now accepts both shapes and reports the canonical ISO-Z spelling — +the repair `@objectstack/metadata-protocol` already carries for `occurred_at`. +Normalising at the driver's read door instead would reverse the deliberate +`withPostgresCalendarDayAsText` decision that a `timestamptz` is an instant, so +the consumer owes the spelling. + +No exported shape changes: `StrandedOrphanSample.createdAt` stays +`string | undefined`. An ISO string is still passed through byte-for-byte, and +an absent, null or unparseable stamp still reports `undefined` — never the +literal `"Invalid Date"` or `"undefined"` in the position an operator reads a +timestamp from. The sibling `key` / `name` guards are untouched: those are text +columns on every dialect, and only the timestamp straddles the divergence. diff --git a/packages/services/service-storage/src/stranded-orphan-inventory.test.ts b/packages/services/service-storage/src/stranded-orphan-inventory.test.ts index fbd1a70401..95d9539972 100644 --- a/packages/services/service-storage/src/stranded-orphan-inventory.test.ts +++ b/packages/services/service-storage/src/stranded-orphan-inventory.test.ts @@ -505,3 +505,138 @@ describe("[#10950] the sweep cannot nominate a stranded orphan — the card's pr expect(report.stranded).toBe(1); }); }); +// ── [#13996] `createdAt` across the dialect divergence ────────────────────── + +/** + * What `samples[].createdAt` is, per runtime shape of `created_at`. + * + * ## Why this file could not have caught the defect before + * + * `created_at` is a BUILTIN audit column, so no declared-field coercion + * reaches it and `SqlDriver#formatOutput` repairs it only inside its + * `if (this.isSqlite)` arm. The record read door therefore hands it back as + * canonical ISO-Z TEXT on SQLite and as a JS `Date` on Postgres and MySQL — + * pinned at that door, per dialect, in driver-sql's + * `sql-driver-13567-audit-stamp-materialisation.test.ts` (§B0/§B1). + * + * ⚠️ This repo's default test backend is SQLite, and every fixture above + * spells `created_at` as an ISO STRING — the one shape the old + * `typeof row.created_at === 'string'` guard accepted. So the guard dropped + * the field for every row on both production default drivers while every pin + * here stayed green. The discriminating input is a `Date`, and until this + * block nothing in this file produced one. + * + * ## What each case is worth as evidence + * + * - POSITIVE is the only case that changes verdict with the repair: red + * before it (`undefined`), green after. + * - CONTROL is a declared REGRESSION CONTROL and ⛔ NOT ablation evidence: + * it is green in both directions by construction, because the SQLite shape + * already worked. It exists to pin that the repair added an accepted shape + * and changed nothing about the one that already round-tripped. + * - REVERSE CONTROL guards the `String(…)` trap: the arm that makes the + * `occurred_at` shape work for a non-optional field would spell the literal + * `"undefined"` / `"Invalid Date"` into an operator's report here. + */ +describe('[#13996] `samples[].createdAt` — the driver-dependent runtime type of `created_at`', () => { + /** Canonical audit-timestamp text: what SQLite stores and `toISOString()` emits. */ + const ISO_Z = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/; + /** Sub-second digits are load-bearing — `String(Date)` drops exactly these. */ + const INSTANT = '2026-08-30T10:19:25.947Z'; + + it('POSITIVE — a JS `Date` (the Postgres/MySQL shape) is reported as canonical ISO-Z text', async () => { + const engine = inventoryEngine({ + files: [strandedRow('os13996_pg', { created_at: new Date(INSTANT) })], + }); + + const report = await inventoryStrandedFileOrphans(engine); + + expect(report.stranded).toBe(1); + expect(report.samples).toHaveLength(1); + const sample = report.samples[0]; + // The whole defect, in one assertion: this was `undefined` for EVERY row + // on both live dialects, for a field the walk explicitly projects. + expect( + sample.createdAt, + 'the driver handed `created_at` out as a Date and the sample dropped it', + ).toBe(INSTANT); + expect(typeof sample.createdAt).toBe('string'); + expect(sample.createdAt).toMatch(ISO_Z); + }); + + it('POSITIVE — it is the `toISOString()` spelling, not `String(Date)`: the milliseconds survive', async () => { + const value = new Date(INSTANT); + expect(value.getMilliseconds(), 'the fixture is vacuous without sub-second digits').toBe(947); + + const report = await inventoryStrandedFileOrphans( + inventoryEngine({ files: [strandedRow('os13996_ms', { created_at: value })] }), + ); + const spelled = report.samples[0].createdAt as string; + + // `String(Date)` renders whole seconds in the PROCESS zone (§A2 of the + // driver pin). Naming the instant exactly is what separates the two. + expect(Date.parse(spelled), 'the reported stamp names the row instant').toBe(value.getTime()); + expect(spelled).not.toBe(String(value)); + expect(Date.parse(String(value))).toBe(value.getTime() - value.getMilliseconds()); + }); + + it('CONTROL (⛔ not ablation evidence — green both directions) — an ISO string is passed through byte-for-byte', async () => { + // The SQLite shape, which already worked. Asserted as the WHOLE sample so + // a change to any neighbouring field would show up here too. + const report = await inventoryStrandedFileOrphans( + inventoryEngine({ files: [strandedRow('os13996_sqlite')] }), + ); + + expect(report.samples).toEqual([ + { + fileId: 'os13996_sqlite', + key: 'attachments/os13996_sqlite.bin', + name: 'os13996_sqlite.bin', + size: 1024, + createdAt: '2026-01-01T00:00:00.000Z', + }, + ]); + }); + + it('CONTROL — passthrough is TOTAL over strings: a non-canonical stamp is not re-parsed or re-spelled', async () => { + // Today any string reaches the report unchanged, including a naive-UTC + // spelling. The repair adds an accepted shape; it must not quietly start + // validating or normalising the shape that already round-tripped. + for (const text of ['2026-01-01 00:00:00', 'not-a-timestamp', '']) { + const report = await inventoryStrandedFileOrphans( + inventoryEngine({ files: [strandedRow('os13996_text', { created_at: text })] }), + ); + expect(report.samples[0].createdAt, `passthrough of ${JSON.stringify(text)}`).toBe(text); + } + }); + + it('REVERSE CONTROL — an absent, null or unusable stamp stays `undefined`, never a spelled-out one', async () => { + const absent = strandedRow('os13996_absent'); + delete (absent as Record).created_at; + + const cases: Array<[string, Record]> = [ + ['absent', absent], + ['null', strandedRow('os13996_null', { created_at: null })], + // `instanceof Date` is TRUE for this one, and `toISOString()` THROWS on + // it — the case that would take the whole inventory down. + ['Invalid Date', strandedRow('os13996_nat', { created_at: new Date('not a date') })], + // Epoch millis: a shape no dialect produces here, kept `undefined` + // rather than stringified into a timestamp position. + ['epoch millis', strandedRow('os13996_num', { created_at: Date.parse(INSTANT) })], + ]; + + for (const [label, row] of cases) { + const report = await inventoryStrandedFileOrphans(inventoryEngine({ files: [row] })); + + expect(report.stranded, `${label}: the row is still inventoried`).toBe(1); + const sample = report.samples[0]; + expect(sample.createdAt, `${label}: an absent stamp must stay absent`).toBeUndefined(); + // Spelled out explicitly: these two strings are what a bare `String(…)` + // terminal arm would put where an operator reads a timestamp. + expect(sample.createdAt).not.toBe('Invalid Date'); + expect(sample.createdAt).not.toBe('undefined'); + // …and dropping the stamp must not drop the row's identity with it. + expect(sample.fileId, `${label}: fileId`).toBe(row.id); + } + }); +}); diff --git a/packages/services/service-storage/src/stranded-orphan-inventory.ts b/packages/services/service-storage/src/stranded-orphan-inventory.ts index 791b997152..e9837b32ba 100644 --- a/packages/services/service-storage/src/stranded-orphan-inventory.ts +++ b/packages/services/service-storage/src/stranded-orphan-inventory.ts @@ -168,6 +168,57 @@ function usableSize(value: unknown): number | undefined { return n; } +/** + * `created_at` as canonical ISO-8601-Z text, whichever shape the driver handed + * it out AS — and `undefined` when the row carries no usable stamp. + * + * ## Why a `typeof v === 'string'` test alone is wrong here + * + * `created_at` is a BUILTIN audit column: it is not in `datetimeFields`, so no + * declared-field coercion reaches it, and `SqlDriver#formatOutput` repairs the + * audit columns only inside its `if (this.isSqlite)` arm. So the read door the + * walk below goes through hands this value back as canonical ISO-Z TEXT on + * SQLite and as a JS `Date` on Postgres and MySQL — the production default + * drivers. Pinned per dialect, at that door, in driver-sql's + * `sql-driver-13567-audit-stamp-materialisation.test.ts` (§B1). + * + * A bare string test therefore answers FALSE for EVERY row on the live + * dialects: a field the walk explicitly projects (`fields: [… 'created_at']`) + * was asked for from the driver and then silently discarded, so every sample + * in the operator's report carried `createdAt: undefined` there while looking + * correct on the SQLite the tests run. The sibling guards on `key` and `name` + * are NOT this — those are text columns on every dialect. Only the timestamp + * straddles the divergence, which is why reading the code did not show it. + * + * ## Why the consumer owes the canonical spelling + * + * Normalising at the driver's read door instead would reverse the deliberate + * `withPostgresCalendarDayAsText` decision — that a `timestamptz` IS an + * instant and a `Date` is the right materialisation for it. So the repair is + * the one `@objectstack/metadata-protocol` already carries for `occurred_at`: + * accept both shapes where the value is consumed. + * + * ⛔ NOT `row.created_at ?? undefined`. That reads as fixed and is worse: it + * puts a raw `Date` into a field declared `string | undefined`, trading a + * dropped field for a wrong type. + * + * ⛔ And the terminal arm is `undefined`, not `String(value)`. This field is + * optional where `occurredAt` is not, and `String()` over a null or an Invalid + * Date spells the literal `"undefined"` / `"Invalid Date"` into an operator's + * report in the position a timestamp is read from — a stamp that is absent + * must stay absent. + */ +function usableCreatedAt(value: unknown): string | undefined { + if (typeof value === 'string') return value; + if (value instanceof Date) { + // An Invalid Date IS `instanceof Date`, and `toISOString()` THROWS on it + // (RangeError) rather than returning something odd. One unparseable stamp + // must not take down a read-only inventory of the whole `sys_file` table. + return Number.isNaN(value.getTime()) ? undefined : value.toISOString(); + } + return undefined; +} + /** * Count the `sys_file` rows that the forward-only fixes (#10171, #10240) would * have tombstoned had they existed when the rows were orphaned, and that the @@ -264,7 +315,7 @@ export async function inventoryStrandedFileOrphans( key: typeof row.key === 'string' ? row.key : undefined, name: typeof row.name === 'string' ? row.name : undefined, size, - createdAt: typeof row.created_at === 'string' ? row.created_at : undefined, + createdAt: usableCreatedAt(row.created_at), }); } }