Skip to content

Commit c75962a

Browse files
claude[bot]claude
andauthored
fix(service-storage): accept the driver's Date for created_at in the stranded-orphan inventory (#14102)
`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, so no declared-field coercion reaches it and `SqlDriver#formatOutput` repairs the audit columns only inside its `if (this.isSqlite)` arm: 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. The guard was therefore false for every row on both live dialects and the projected field was silently discarded from every sample. Accept both shapes at the consumer, in the form `@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 canonical spelling. The terminal arm stays `undefined` rather than `String(value)`: this field is optional where `occurredAt` is not, and stringifying a null or an Invalid Date would spell `"undefined"` / `"Invalid Date"` into the position an operator reads a timestamp from. The sibling `key` / `name` guards are untouched — those are text columns on every dialect. Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs Co-authored-by: Claude <noreply@anthropic.com>
1 parent b8c4762 commit c75962a

3 files changed

Lines changed: 219 additions & 1 deletion

File tree

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
---
2+
"@objectstack/service-storage": patch
3+
---
4+
5+
fix(service-storage): report `createdAt` on every stranded-orphan sample, not only on SQLite (#13996)
6+
7+
`inventoryStrandedFileOrphans` projects `created_at` out of the `sys_file` read
8+
door and then tested it with `typeof row.created_at === 'string'`. `created_at`
9+
is a BUILTIN audit column — it is not in `datetimeFields`, and
10+
`SqlDriver#formatOutput` repairs the audit columns only inside its
11+
`if (this.isSqlite)` arm — so that door hands the value back as canonical ISO-Z
12+
text on SQLite and as a JS `Date` on Postgres and MySQL, the production default
13+
drivers (pinned per dialect in driver-sql's
14+
`sql-driver-13567-audit-stamp-materialisation.test.ts`).
15+
16+
The guard was therefore `false` for **every** row on both live dialects: a field
17+
explicitly asked for from the driver was silently discarded, and every sample in
18+
an operator's stranded-orphan report carried `createdAt: undefined` there while
19+
looking correct on the SQLite the suite runs on.
20+
21+
The consumer now accepts both shapes and reports the canonical ISO-Z spelling —
22+
the repair `@objectstack/metadata-protocol` already carries for `occurred_at`.
23+
Normalising at the driver's read door instead would reverse the deliberate
24+
`withPostgresCalendarDayAsText` decision that a `timestamptz` is an instant, so
25+
the consumer owes the spelling.
26+
27+
No exported shape changes: `StrandedOrphanSample.createdAt` stays
28+
`string | undefined`. An ISO string is still passed through byte-for-byte, and
29+
an absent, null or unparseable stamp still reports `undefined` — never the
30+
literal `"Invalid Date"` or `"undefined"` in the position an operator reads a
31+
timestamp from. The sibling `key` / `name` guards are untouched: those are text
32+
columns on every dialect, and only the timestamp straddles the divergence.

‎packages/services/service-storage/src/stranded-orphan-inventory.test.ts‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -505,3 +505,138 @@ describe("[#10950] the sweep cannot nominate a stranded orphan — the card's pr
505505
expect(report.stranded).toBe(1);
506506
});
507507
});
508+
// ── [#13996] `createdAt` across the dialect divergence ──────────────────────
509+
510+
/**
511+
* What `samples[].createdAt` is, per runtime shape of `created_at`.
512+
*
513+
* ## Why this file could not have caught the defect before
514+
*
515+
* `created_at` is a BUILTIN audit column, so no declared-field coercion
516+
* reaches it and `SqlDriver#formatOutput` repairs it only inside its
517+
* `if (this.isSqlite)` arm. The record read door therefore hands it back as
518+
* canonical ISO-Z TEXT on SQLite and as a JS `Date` on Postgres and MySQL —
519+
* pinned at that door, per dialect, in driver-sql's
520+
* `sql-driver-13567-audit-stamp-materialisation.test.ts` (§B0/§B1).
521+
*
522+
* ⚠️ This repo's default test backend is SQLite, and every fixture above
523+
* spells `created_at` as an ISO STRING — the one shape the old
524+
* `typeof row.created_at === 'string'` guard accepted. So the guard dropped
525+
* the field for every row on both production default drivers while every pin
526+
* here stayed green. The discriminating input is a `Date`, and until this
527+
* block nothing in this file produced one.
528+
*
529+
* ## What each case is worth as evidence
530+
*
531+
* - POSITIVE is the only case that changes verdict with the repair: red
532+
* before it (`undefined`), green after.
533+
* - CONTROL is a declared REGRESSION CONTROL and ⛔ NOT ablation evidence:
534+
* it is green in both directions by construction, because the SQLite shape
535+
* already worked. It exists to pin that the repair added an accepted shape
536+
* and changed nothing about the one that already round-tripped.
537+
* - REVERSE CONTROL guards the `String(…)` trap: the arm that makes the
538+
* `occurred_at` shape work for a non-optional field would spell the literal
539+
* `"undefined"` / `"Invalid Date"` into an operator's report here.
540+
*/
541+
describe('[#13996] `samples[].createdAt` — the driver-dependent runtime type of `created_at`', () => {
542+
/** Canonical audit-timestamp text: what SQLite stores and `toISOString()` emits. */
543+
const ISO_Z = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/;
544+
/** Sub-second digits are load-bearing — `String(Date)` drops exactly these. */
545+
const INSTANT = '2026-08-30T10:19:25.947Z';
546+
547+
it('POSITIVE — a JS `Date` (the Postgres/MySQL shape) is reported as canonical ISO-Z text', async () => {
548+
const engine = inventoryEngine({
549+
files: [strandedRow('os13996_pg', { created_at: new Date(INSTANT) })],
550+
});
551+
552+
const report = await inventoryStrandedFileOrphans(engine);
553+
554+
expect(report.stranded).toBe(1);
555+
expect(report.samples).toHaveLength(1);
556+
const sample = report.samples[0];
557+
// The whole defect, in one assertion: this was `undefined` for EVERY row
558+
// on both live dialects, for a field the walk explicitly projects.
559+
expect(
560+
sample.createdAt,
561+
'the driver handed `created_at` out as a Date and the sample dropped it',
562+
).toBe(INSTANT);
563+
expect(typeof sample.createdAt).toBe('string');
564+
expect(sample.createdAt).toMatch(ISO_Z);
565+
});
566+
567+
it('POSITIVE — it is the `toISOString()` spelling, not `String(Date)`: the milliseconds survive', async () => {
568+
const value = new Date(INSTANT);
569+
expect(value.getMilliseconds(), 'the fixture is vacuous without sub-second digits').toBe(947);
570+
571+
const report = await inventoryStrandedFileOrphans(
572+
inventoryEngine({ files: [strandedRow('os13996_ms', { created_at: value })] }),
573+
);
574+
const spelled = report.samples[0].createdAt as string;
575+
576+
// `String(Date)` renders whole seconds in the PROCESS zone (§A2 of the
577+
// driver pin). Naming the instant exactly is what separates the two.
578+
expect(Date.parse(spelled), 'the reported stamp names the row instant').toBe(value.getTime());
579+
expect(spelled).not.toBe(String(value));
580+
expect(Date.parse(String(value))).toBe(value.getTime() - value.getMilliseconds());
581+
});
582+
583+
it('CONTROL (⛔ not ablation evidence — green both directions) — an ISO string is passed through byte-for-byte', async () => {
584+
// The SQLite shape, which already worked. Asserted as the WHOLE sample so
585+
// a change to any neighbouring field would show up here too.
586+
const report = await inventoryStrandedFileOrphans(
587+
inventoryEngine({ files: [strandedRow('os13996_sqlite')] }),
588+
);
589+
590+
expect(report.samples).toEqual([
591+
{
592+
fileId: 'os13996_sqlite',
593+
key: 'attachments/os13996_sqlite.bin',
594+
name: 'os13996_sqlite.bin',
595+
size: 1024,
596+
createdAt: '2026-01-01T00:00:00.000Z',
597+
},
598+
]);
599+
});
600+
601+
it('CONTROL — passthrough is TOTAL over strings: a non-canonical stamp is not re-parsed or re-spelled', async () => {
602+
// Today any string reaches the report unchanged, including a naive-UTC
603+
// spelling. The repair adds an accepted shape; it must not quietly start
604+
// validating or normalising the shape that already round-tripped.
605+
for (const text of ['2026-01-01 00:00:00', 'not-a-timestamp', '']) {
606+
const report = await inventoryStrandedFileOrphans(
607+
inventoryEngine({ files: [strandedRow('os13996_text', { created_at: text })] }),
608+
);
609+
expect(report.samples[0].createdAt, `passthrough of ${JSON.stringify(text)}`).toBe(text);
610+
}
611+
});
612+
613+
it('REVERSE CONTROL — an absent, null or unusable stamp stays `undefined`, never a spelled-out one', async () => {
614+
const absent = strandedRow('os13996_absent');
615+
delete (absent as Record<string, unknown>).created_at;
616+
617+
const cases: Array<[string, Record<string, unknown>]> = [
618+
['absent', absent],
619+
['null', strandedRow('os13996_null', { created_at: null })],
620+
// `instanceof Date` is TRUE for this one, and `toISOString()` THROWS on
621+
// it — the case that would take the whole inventory down.
622+
['Invalid Date', strandedRow('os13996_nat', { created_at: new Date('not a date') })],
623+
// Epoch millis: a shape no dialect produces here, kept `undefined`
624+
// rather than stringified into a timestamp position.
625+
['epoch millis', strandedRow('os13996_num', { created_at: Date.parse(INSTANT) })],
626+
];
627+
628+
for (const [label, row] of cases) {
629+
const report = await inventoryStrandedFileOrphans(inventoryEngine({ files: [row] }));
630+
631+
expect(report.stranded, `${label}: the row is still inventoried`).toBe(1);
632+
const sample = report.samples[0];
633+
expect(sample.createdAt, `${label}: an absent stamp must stay absent`).toBeUndefined();
634+
// Spelled out explicitly: these two strings are what a bare `String(…)`
635+
// terminal arm would put where an operator reads a timestamp.
636+
expect(sample.createdAt).not.toBe('Invalid Date');
637+
expect(sample.createdAt).not.toBe('undefined');
638+
// …and dropping the stamp must not drop the row's identity with it.
639+
expect(sample.fileId, `${label}: fileId`).toBe(row.id);
640+
}
641+
});
642+
});

‎packages/services/service-storage/src/stranded-orphan-inventory.ts‎

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,57 @@ function usableSize(value: unknown): number | undefined {
168168
return n;
169169
}
170170

171+
/**
172+
* `created_at` as canonical ISO-8601-Z text, whichever shape the driver handed
173+
* it out AS — and `undefined` when the row carries no usable stamp.
174+
*
175+
* ## Why a `typeof v === 'string'` test alone is wrong here
176+
*
177+
* `created_at` is a BUILTIN audit column: it is not in `datetimeFields`, so no
178+
* declared-field coercion reaches it, and `SqlDriver#formatOutput` repairs the
179+
* audit columns only inside its `if (this.isSqlite)` arm. So the read door the
180+
* walk below goes through hands this value back as canonical ISO-Z TEXT on
181+
* SQLite and as a JS `Date` on Postgres and MySQL — the production default
182+
* drivers. Pinned per dialect, at that door, in driver-sql's
183+
* `sql-driver-13567-audit-stamp-materialisation.test.ts` (§B1).
184+
*
185+
* A bare string test therefore answers FALSE for EVERY row on the live
186+
* dialects: a field the walk explicitly projects (`fields: [… 'created_at']`)
187+
* was asked for from the driver and then silently discarded, so every sample
188+
* in the operator's report carried `createdAt: undefined` there while looking
189+
* correct on the SQLite the tests run. The sibling guards on `key` and `name`
190+
* are NOT this — those are text columns on every dialect. Only the timestamp
191+
* straddles the divergence, which is why reading the code did not show it.
192+
*
193+
* ## Why the consumer owes the canonical spelling
194+
*
195+
* Normalising at the driver's read door instead would reverse the deliberate
196+
* `withPostgresCalendarDayAsText` decision — that a `timestamptz` IS an
197+
* instant and a `Date` is the right materialisation for it. So the repair is
198+
* the one `@objectstack/metadata-protocol` already carries for `occurred_at`:
199+
* accept both shapes where the value is consumed.
200+
*
201+
* ⛔ NOT `row.created_at ?? undefined`. That reads as fixed and is worse: it
202+
* puts a raw `Date` into a field declared `string | undefined`, trading a
203+
* dropped field for a wrong type.
204+
*
205+
* ⛔ And the terminal arm is `undefined`, not `String(value)`. This field is
206+
* optional where `occurredAt` is not, and `String()` over a null or an Invalid
207+
* Date spells the literal `"undefined"` / `"Invalid Date"` into an operator's
208+
* report in the position a timestamp is read from — a stamp that is absent
209+
* must stay absent.
210+
*/
211+
function usableCreatedAt(value: unknown): string | undefined {
212+
if (typeof value === 'string') return value;
213+
if (value instanceof Date) {
214+
// An Invalid Date IS `instanceof Date`, and `toISOString()` THROWS on it
215+
// (RangeError) rather than returning something odd. One unparseable stamp
216+
// must not take down a read-only inventory of the whole `sys_file` table.
217+
return Number.isNaN(value.getTime()) ? undefined : value.toISOString();
218+
}
219+
return undefined;
220+
}
221+
171222
/**
172223
* Count the `sys_file` rows that the forward-only fixes (#10171, #10240) would
173224
* have tombstoned had they existed when the rows were orphaned, and that the
@@ -264,7 +315,7 @@ export async function inventoryStrandedFileOrphans(
264315
key: typeof row.key === 'string' ? row.key : undefined,
265316
name: typeof row.name === 'string' ? row.name : undefined,
266317
size,
267-
createdAt: typeof row.created_at === 'string' ? row.created_at : undefined,
318+
createdAt: usableCreatedAt(row.created_at),
268319
});
269320
}
270321
}

0 commit comments

Comments
 (0)