|
| 1 | +--- |
| 2 | +"@objectstack/metadata": minor |
| 3 | +"@objectstack/metadata-protocol": patch |
| 4 | +--- |
| 5 | + |
| 6 | +fix(metadata): four `isoFromValidDate` call sites collapse onto the shared canonical-ISO spelling; `MetadataHistoryRecord.recordedAt` gets the terminal value it never had (#16422) |
| 7 | + |
| 8 | +## What was wrong |
| 9 | + |
| 10 | +`#14037`/`#14038` landed a narrow per-site helper, `isoFromValidDate`, beside |
| 11 | +the shared `canonicalIsoInstant` spelling. It rewrote exactly one shape — a |
| 12 | +valid JS `Date` becomes ISO text — and handed **every other input back |
| 13 | +untouched**. Four adapter boundaries used it, and each fed a field declared |
| 14 | +`z.string()` or `z.string().datetime()`: |
| 15 | + |
| 16 | +| site | declared as | |
| 17 | +|:--|:--| |
| 18 | +| `SysMetadataRepository.rowToEvent` → `MetadataEvent.ts` | `z.string()` | |
| 19 | +| `DatabaseLoader.rowToRecord` → `MetadataRecord.createdAt` / `.updatedAt` | `z.string().datetime().optional()` | |
| 20 | +| `DatabaseLoader.getHistoryRecord` → `MetadataHistoryRecord.recordedAt` | `z.string().datetime()` — **required** | |
| 21 | +| `DatabaseLoader.queryHistory` → the same field, the other door | `z.string().datetime()` — **required** | |
| 22 | + |
| 23 | +So a `null`, a `number`, an opaque column and an Invalid `Date` all arrived at a |
| 24 | +field declared `string`, each wearing an `as string` / `as string | undefined` |
| 25 | +cast that asserted the opposite. Measured over the seven inputs that |
| 26 | +distinguish the two helpers, the declared schemas refused **21 of 35** produced |
| 27 | +values. |
| 28 | + |
| 29 | +`recordedAt` was the sharp end: a REQUIRED `z.string().datetime()` for which |
| 30 | +none of the three available answers was legal — the visible text |
| 31 | +`"Invalid Date"` fails the refinement, `undefined` fails the required field, and |
| 32 | +the pass-through fed it the `Date` object, which fails both. |
| 33 | + |
| 34 | +## What it does now |
| 35 | + |
| 36 | +Those four sites read `canonicalIsoInstant`, whose return type **is** |
| 37 | +`string | undefined`, so all four casts are deleted rather than restated. Both |
| 38 | +sibling definitions of `isoFromValidDate` are gone. The terminal value is chosen |
| 39 | +per site, from the site's own declared schema: |
| 40 | + |
| 41 | +- `MetadataRecord.createdAt` / `.updatedAt` are `.optional()` → `undefined`, the |
| 42 | + branch an absent column already took. ⛔ No default is invented for a field the |
| 43 | + schema lets be absent. |
| 44 | +- `MetadataHistoryRecord.recordedAt` is required → the **epoch**, via a named |
| 45 | + `recordedAtFallback()` shared by both history doors. ⛔ Not `new Date()`: a |
| 46 | + `now` stamp is a plausible-looking recording instant nobody measured, and it |
| 47 | + sorts a version recorded years ago to the top of a newest-first timeline. The |
| 48 | + epoch invents no fact and sorts to the oldest end. It is also the answer the |
| 49 | + sibling reader of this same `sys_metadata_history.recorded_at` column already |
| 50 | + gives (`rowToEvent` and `history()`, both `?? new Date(0).toISOString()`). |
| 51 | + |
| 52 | +Schema refusals over the same seven inputs: **21 → 8**. The eight that remain |
| 53 | +are a `number` and an opaque object at four sites — shapes no driver is measured |
| 54 | +to materialise for these columns. They now arrive as the declared *type* (a |
| 55 | +string) that simply is not a valid datetime, so the producer's bug stays visible |
| 56 | +instead of being papered over. |
| 57 | + |
| 58 | +## One behaviour change worth reading twice — and it is why this is `minor` |
| 59 | + |
| 60 | +`DatabaseLoader.stat()` computes `record.updatedAt ?? record.createdAt`. An |
| 61 | +Invalid `updated_at` used to WIN that `??` — a `Date` is truthy and not nullish — |
| 62 | +so a row with an unreadable `updated_at` and a good `created_at` published |
| 63 | +`new Date()` as its `mtime`. It now folds to `undefined` one step earlier and |
| 64 | +loses the `??`, so the row publishes its `created_at`: a stored instant in place |
| 65 | +of a fabricated one, and exactly the "same `?? DEFAULT` chain an absent column |
| 66 | +takes" that `#14078`'s own ruling text prescribes for the shape. |
| 67 | + |
| 68 | +⚠️ **The old answer was LEGAL.** `new Date().toISOString()` satisfies |
| 69 | +`MetadataStats.mtime`'s `z.string().datetime()` perfectly well, and the |
| 70 | +pre-existing pin asserted exactly that. So this one site is **not** the repair of |
| 71 | +a violation — it is one legal published answer replaced by a different legal |
| 72 | +published answer on a published read verb. Nothing was refused before and is |
| 73 | +permitted now; a consumer simply receives a different instant. |
| 74 | + |
| 75 | +## Why the two levels differ |
| 76 | + |
| 77 | +- **`@objectstack/metadata` — `minor`.** Its four repaired sites, on their own, |
| 78 | + are the "repairing an implementation that silently violated its own already |
| 79 | + published declared type" case: the values that changed there are ones |
| 80 | + `MetadataRecordSchema` / `MetadataHistoryRecordSchema` already refused, and |
| 81 | + nothing a consumer legitimately received has moved. But this package also |
| 82 | + carries `stat()`, and that site changes a **legal** published answer, which the |
| 83 | + paragraph above measures. The level is per package, so the four repaired sites |
| 84 | + ride along at `minor`. |
| 85 | +- **`@objectstack/metadata-protocol` — `patch`.** Neither of its two sites moves |
| 86 | + a legal published answer. `rowToEvent` only stops emitting values |
| 87 | + `MetadataEventSchema` refused (a `Date`, a `number`, an opaque object in a |
| 88 | + field declared `z.string()`), and `listCommits` is byte-identical on all seven |
| 89 | + probe inputs. |
| 90 | + |
| 91 | +⛔ No declared type narrowed, no export was added or removed (neither helper was |
| 92 | +ever exported), and no envelope or accept set moved — so this is `minor` by the |
| 93 | +changed-answer row, not a breaking change, and it carries no ADR-0087 |
| 94 | +disposition. |
| 95 | + |
| 96 | +## What deliberately did NOT collapse |
| 97 | + |
| 98 | +`listCommits` in `@objectstack/metadata-protocol` keeps its copy. Its docblock |
| 99 | +promises callers the RAW value back for a non-`Date`, and the shared spelling |
| 100 | +rewrites the whole domain: swapping it in would ERASE an Invalid `Date` from the |
| 101 | +response (`undefined` — the one answer ADR-0053 D-F3 refuses, because it silently |
| 102 | +drops a value that is on disk) and hand a `number` or an opaque object to the |
| 103 | +commit-timeline sort as `String(value)` rather than verbatim. Measured, that site |
| 104 | +is byte-identical on all seven inputs before and after this change. |
| 105 | + |
| 106 | +`SqlDriver`'s same-named helper is not part of this family at all: it takes |
| 107 | +`Date` (not `unknown`), both its call sites narrow with `instanceof Date` first, |
| 108 | +and it is the PRODUCER-side fold ADR-0053 D-F3 governs. It is untouched. |
0 commit comments