diff --git a/.changeset/invalid-date-total-arm-metadata-protocol.md b/.changeset/invalid-date-total-arm-metadata-protocol.md new file mode 100644 index 0000000000..cf35a4c262 --- /dev/null +++ b/.changeset/invalid-date-total-arm-metadata-protocol.md @@ -0,0 +1,16 @@ +--- +'@objectstack/metadata-protocol': patch +--- + +Serve an Invalid `Date` from a driver instead of raising `RangeError` at two metadata read seams. + +`canonicalIsoInstant` (`sys-metadata-repository.ts`) and the `occurredAt` arm inside `auditMetaItem` (`protocol.ts`) both reached `value.toISOString()` for any `Date`. That call raises `RangeError: Invalid time value` for the one `Date` whose time value is `NaN`, so a single bad row answered **500** on a read path — where the spelling these repairs replaced, `String(value)`, had served a visibly-wrong field the caller could see and report. + +The shape is measured, not hypothetical: mysql2 3.23.1 returns a module constant literally named `INVALID_DATE` for a zero `DATETIME`, and postgres-date 1.0.7 builds `new Date(NaN)` for every year in 275760..294276 — a range Postgres itself stores. Legacy imports, hand migrations and a MySQL database shared with another application are all ordinary ways such a row arrives. + +Both arms now guard on `Number.isNaN(value.getTime())`, and the terminal value is chosen per call site rather than uniformly: + +- `canonicalIsoInstant` answers `undefined`, so each caller's existing `?? ` chain — the branch an absent column already takes — keeps its meaning. Its consumers are machines, and one forwards into a `z.string().datetime()` field that visible text would fail. +- `auditMetaItem`'s `occurredAt` falls into the `String(...)` arm already beside it, which renders exactly `"Invalid Date"`. `AuditMetaItemResponseSchema.events[].occurredAt` is a required plain `z.string()` read by an operator in Studio's audit tab, so the text satisfies the contract and one bad row no longer blanks the page. + +Neither answer is a blank: a silent empty value is the shape that hides the producer's bug. diff --git a/.changeset/invalid-date-total-arm-metadata.md b/.changeset/invalid-date-total-arm-metadata.md new file mode 100644 index 0000000000..c8ca029b27 --- /dev/null +++ b/.changeset/invalid-date-total-arm-metadata.md @@ -0,0 +1,11 @@ +--- +'@objectstack/metadata': patch +--- + +Serve an Invalid `Date` from a driver instead of raising `RangeError` in `DatabaseLoader.stat`. + +`canonicalIsoInstant` reached `value.toISOString()` for any `Date`, and that call raises `RangeError: Invalid time value` for the one `Date` whose time value is `NaN`. `stat()` is a hot read path — REST `/meta/*`, ObjectQL plan resolution, runtime overlay merges — so one legacy `sys_metadata` row answered **500** where the spelling this repair replaced had served a visibly-wrong value. + +The shape is measured: mysql2 3.23.1 hands back a constant literally named `INVALID_DATE` for a zero `DATETIME`, and postgres-date 1.0.7 builds `new Date(NaN)` for every year in 275760..294276, which Postgres itself stores. + +The `Date` arm now guards on `Number.isNaN(value.getTime())` and answers `undefined`, so `stat()`'s own `?? new Date().toISOString()` — the branch an absent column already takes — publishes a parseable `MetadataStats.mtime`. `undefined` rather than visible text is deliberate here: `mtime` is declared `z.string().datetime()`, so the text `"Invalid Date"` would not produce a readable cell, it would produce a zod refusal at the consumer, moving the failure instead of removing it. A blank is excluded for the opposite reason — it hides the producer's bug. diff --git a/.changeset/invalid-date-total-arm-rest.md b/.changeset/invalid-date-total-arm-rest.md new file mode 100644 index 0000000000..ca21d3a2a4 --- /dev/null +++ b/.changeset/invalid-date-total-arm-rest.md @@ -0,0 +1,13 @@ +--- +'@objectstack/rest': patch +--- + +Serve an Invalid `Date` from a driver as visible text instead of raising `RangeError` in the import-job DTO and the CSV export. + +`canonicalIsoStamp` and `formatCsvCell` both reached `value.toISOString()` for any `Date`, and that call raises `RangeError: Invalid time value` for the one `Date` whose time value is `NaN` — so one bad timestamp column answered **500** on `GET /api/v1/data/import/jobs/:jobId` and aborted a CSV export mid-stream. + +The shape is measured: mysql2 3.23.1 returns a module constant literally named `INVALID_DATE` for a zero `DATETIME`, and postgres-date 1.0.7 builds `new Date(NaN)` for every year in 275760..294276, a range Postgres itself stores. + +Both arms now guard on `Number.isNaN(value.getTime())` and render the visible text `"Invalid Date"` — the rendering the spelling they replaced produced. Both are read by a human, and the declared contracts allow it: the four import-job stamps are plain `z.string()` (not `z.string().datetime()`), and a CSV cell has no schema at all. The operator sees a wrong-looking field they can report, rather than an error naming no row. + +The CSV arm needs its own guard rather than a fall-through, because the branch below it is `JSON.stringify` and `Date.prototype.toJSON` answers `null` for an Invalid `Date` — the silent blank this change exists to avoid. Both CSV paths land on the guarded arm: with field metadata, `formatDate` rejects an Invalid `Date` and passes the value through unchanged. diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 8640ef74a9..996d2c325f 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -64,7 +64,7 @@ not on any flag. ## How the flag is set `isSystem` is **server-constructed and never client-supplied**. Inbound HTTP -cannot set it (`packages/rest/src/rest-server.ts:1749`, `:1778`), and neither +cannot set it (`packages/rest/src/rest-server.ts:1788`, `:1817`), and neither can an action body (`packages/runtime/src/domains/actions.ts:414`). It is written by internal callers only, as an option on the engine call: @@ -103,7 +103,7 @@ that silently does not happen. | 14 | MCP stdio bridge skips the object API-exposure gate | mcp | Get: the bridge reaches objects whose `apiEnabled` / `apiMethods` would refuse an external caller | `stdio-data-bridge.ts:250` | | 15 | **Read-audit rows are not written** | plugin-audit | Lose: the "a person opened this record" trail. `sudo()` keeps the caller's `userId`, so this flag is the only thing separating a human read from a platform one | `read-audit.ts:556` | | 16 | Approval snapshot payload redaction skipped | plugin-approvals | Get: the whole snapshot on `find` / `findOne` — the audit/replay channel. Lose: field-visibility redaction over approval payloads | `payload-redaction-middleware.ts:115` | -| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1781` | +| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1820` | ### 2. Write pipeline and data integrity @@ -158,7 +158,7 @@ The largest single consumer — **17 of the 105 sites**. |:--|:---|:---|:---|:---| | 48 | Object API-exposure gate bypassed (`apiEnabled` / `apiMethods`) | runtime | Get: internal self-writes ignore exposure declarations — these govern **external** exposure, not engine self-writes | `action-execution.ts:138` | | 49 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `action-execution.ts:401` | -| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:5411`, `:6868`, `:7116`, `:7547`, `:7740` | +| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:5450`, `:6907`, `:7155`, `:7586`, `:7779` | | 51 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 50's doors consult answers yes before any capability is examined | `meta-write-capability.ts:134` | | 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:421`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:552`, `external-datasource-routes.ts:302`, `package-routes.ts:97` | | 53 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `domains/mcp.ts:61` | @@ -199,7 +199,7 @@ assuming `isSystem` covers it is a documented source of bugs. | "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1590` (#3493 / #6640) | | "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280`–`281` | | "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:299` | -| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1749`, `:1778`; `domains/actions.ts:414` | +| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1788`, `:1817`; `domains/actions.ts:414` | --- diff --git a/packages/metadata-protocol/src/protocol-14038-list-commits-created-at-iso.test.ts b/packages/metadata-protocol/src/protocol-14038-list-commits-created-at-iso.test.ts index 77b79df842..302f9814d9 100644 --- a/packages/metadata-protocol/src/protocol-14038-list-commits-created-at-iso.test.ts +++ b/packages/metadata-protocol/src/protocol-14038-list-commits-created-at-iso.test.ts @@ -35,13 +35,15 @@ * did NOT adopt `canonicalIsoInstant` (`sys-metadata-repository.ts` / * `database-loader.ts`), because #14078 measured an Invalid `Date` reachable * on BOTH live dialects (a MySQL zero datetime; any Postgres year in - * 275760..294276) where that spelling's `value.toISOString()` raises - * `RangeError`, and #13973 is `pm:blocked` on that ruling. This card follows - * #14037's precedent: `isoFromValidDate` in `protocol.ts` converts the ONE - * measured shape (a valid `Date`) and returns every other shape — including - * an Invalid `Date` — UNCHANGED. §D below is the pin that this card does not - * decide #14078: it goes red the moment anyone swaps the contested spelling - * into this site. + * 275760..294276) where that spelling's `value.toISOString()` raised + * `RangeError`. #14078 has since ruled (option B, 2026-09-02) and that arm is + * now total, answering `undefined` for the shape. This card's route is + * unchanged: `isoFromValidDate` in `protocol.ts` converts the ONE measured + * shape (a valid `Date`) and returns every other shape — including an Invalid + * `Date` — UNCHANGED, which is what `listCommits` promises its callers. §D + * below stays the pin on that promise: it goes red the moment anyone swaps + * the other spelling into this site, now the separately-tracked consolidation + * decision #16422. * * ## Reverse verification, direction predicted BEFORE running * @@ -160,12 +162,13 @@ describe('[#14038] listCommits emits the ISO-8601 string createdAt is declared a * Postgres year in 275760..294276), and whether the shared * canonical-ISO spelling (`canonicalIsoInstant`) should throw on it * (option A) or fall back to a rendering (option B) is a maintainer - * call across four packages. Until it is ruled, this site hands that - * one shape through exactly as it does today — no new throw, no + * call across four packages; it was ruled B on 2026-09-02 for the + * five arms that THREW, and this site was not one of them. It hands + * that one shape through exactly as it does today — no new throw, no * invented rendering. This case is what makes that a PIN rather than * a claim: it goes red the moment `canonicalIsoInstant` (or any * spelling that reaches `.toISOString()` unconditionally) is swapped - * into `listCommits`. + * into `listCommits`. The consolidation is #16422. */ it('hands the value through unchanged instead of raising RangeError', async () => { const invalid = new Date(NaN); diff --git a/packages/metadata-protocol/src/protocol-14078-audit-invalid-date-total-arm.test.ts b/packages/metadata-protocol/src/protocol-14078-audit-invalid-date-total-arm.test.ts new file mode 100644 index 0000000000..f6c3bda865 --- /dev/null +++ b/packages/metadata-protocol/src/protocol-14078-audit-invalid-date-total-arm.test.ts @@ -0,0 +1,148 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14078] `auditMetaItem`'s `occurredAt` arm is TOTAL — an Invalid `Date` + * renders as the visible text `"Invalid Date"`, never as a `RangeError`. + * + * ## The defect + * + * The mapping was a ternary whose middle arm ran `r.occurred_at.toISOString()` + * for ANY `Date`. `toISOString()` raises `RangeError: Invalid time value` on a + * `Date` whose time value is `NaN`, so ONE bad `sys_metadata_audit` row turned + * `GET /api/v1/meta/:type/:name/audit` — the read behind Studio's 审计日志 tab — + * into a 500 for the whole page, on a row the error does not name. This is a + * COMPLIANCE surface: the trail going dark is the failure mode it exists to + * prevent. + * + * ## Reachability is measured, not argued + * + * PR #14409 (landed `3ecb7dc1a`): mysql2 3.23.1 returns a module constant + * literally named `INVALID_DATE` for a zero `DATETIME`; postgres-date 1.0.7 + * builds `new Date(NaN)` for every year in 275760..294276, a range Postgres + * itself stores. The maintainer ruled option B on 2026-09-02, on all five arms + * of the shared spelling at once. + * + * ## Why the terminal value here is the TEXT, not `undefined` + * + * The ruling sets it per call site: visible text where the field is required + * and an operator reads it. `AuditMetaItemResponseSchema.events[].occurredAt` + * is a REQUIRED plain `z.string()` — not `z.string().datetime()` — so the text + * satisfies the declared contract and arrives in the tab where a human can see + * and report it. `undefined` would fail the required field, and a blank `''` + * is the silent shape the ruling forbids by name. + * + * The value is reached by letting the guard fail into the `String(...)` arm + * that was already there, so the rendering is literally the one the pre-repair + * spelling produced. §A pins that identity rather than only the literal. + * + * ## What makes these cases non-vacuous + * + * Every case proves its planted value is a `Date` with a `NaN` time value and + * evaluates the OLD arm's expression on that same object, asserting it raises + * `RangeError`. §B is the discrimination limb: a valid `Date` is still + * canonicalised and a canonical string is still a fixed point, so the guard + * cannot pass by having disabled the arm it guards. + */ + +import { describe, it, expect } from 'vitest'; +import { AuditMetaItemResponseSchema } from '@objectstack/spec/api'; +import { ObjectStackProtocolImplementation } from './protocol.js'; + +/** Canonical ISO-8601 UTC with milliseconds. */ +const ISO_Z = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/; + +/** Non-zero milliseconds, so a truncating regression stays observable. */ +const PG_INSTANT = new Date('2026-03-04T05:06:07.089Z'); +const SQLITE_TEXT = '2026-03-04T05:06:07.089Z'; + +/** + * The removed guard, reproduced: the OLD arm's expression on the very object + * the case plants. Red here means the fixture is no longer the contested shape. + */ +function assertOldSpellingWouldThrow(value: Date): void { + expect(value, 'fixture degraded — not a Date').toBeInstanceOf(Date); + expect(Number.isNaN(value.getTime()), 'fixture is a VALID Date — case is vacuous').toBe(true); + expect(() => value.toISOString()).toThrow(RangeError); +} + +function auditRow(stamp: unknown): Record { + return { + id: 'aud_1', + occurred_at: stamp, + actor: 'usr_1', + source: 'protocol.saveMetaItem', + operation: 'save', + outcome: 'allowed', + code: 'ok', + lock_state: null, + lock_overridden: false, + request_id: 'req_1', + note: null, + }; +} + +/** The real `auditMetaItem`, over an engine whose read door returns `rows`. */ +function protocolOver(rows: Array>) { + const engine = { registry: { getObject: () => undefined }, find: async () => rows }; + return new ObjectStackProtocolImplementation(engine as never); +} + +const REQ = { type: 'views', name: 'case_grid' }; + +describe('[#14078] §A an Invalid Date is served as visible text, not a 500', () => { + it('renders `Invalid Date` and satisfies the declared response contract', async () => { + const bad = new Date(NaN); + assertOldSpellingWouldThrow(bad); + + const res = await protocolOver([auditRow(bad)]); + const body = await res.auditMetaItem(REQ); + + expect(body.events).toHaveLength(1); + expect(body.events[0]!.occurredAt).toBe('Invalid Date'); + + // The rendering is the pre-repair spelling's own, not a literal invented + // here: `String(new Date(NaN))` is `"Invalid Date"` by ECMA-262. + expect(body.events[0]!.occurredAt).toBe(String(bad)); + + // ⛔ The blank the ruling forbids by name. + expect(body.events[0]!.occurredAt).not.toBe(''); + + // The contract itself — a REQUIRED plain `z.string()`, so the text passes + // and reaches the operator's tab. + const parsed = AuditMetaItemResponseSchema.safeParse(body); + expect(parsed.success, JSON.stringify((parsed as { error?: { issues: unknown } }).error?.issues)).toBe(true); + }); + + it('keeps the REST of the trail readable — one bad row does not blank the page', async () => { + const bad = new Date(NaN); + assertOldSpellingWouldThrow(bad); + + const body = await protocolOver([auditRow(bad), auditRow(PG_INSTANT)]).auditMetaItem(REQ); + + // The whole point of the ruling: the good rows survive the bad one. + expect(body.events.map((e) => e.occurredAt)).toEqual(['Invalid Date', PG_INSTANT.toISOString()]); + expect(AuditMetaItemResponseSchema.safeParse(body).success).toBe(true); + }); +}); + +describe('[#14078] §B the guard discriminates — the arm it guards still works', () => { + it('canonicalises a VALID Date byte-exactly', async () => { + const body = await protocolOver([auditRow(PG_INSTANT)]).auditMetaItem(REQ); + expect(body.events[0]!.occurredAt).toBe(PG_INSTANT.toISOString()); + expect(body.events[0]!.occurredAt).toMatch(ISO_Z); + }); + + it('leaves an already-canonical SQLite string byte-identical', async () => { + const body = await protocolOver([auditRow(SQLITE_TEXT)]).auditMetaItem(REQ); + expect(body.events[0]!.occurredAt).toBe(SQLITE_TEXT); + }); + + it('still renders an absent column as the empty string it always did', async () => { + const row = auditRow(null); + delete row.occurred_at; + const body = await protocolOver([row]).auditMetaItem(REQ); + // Unchanged by this card — the nullish arm's meaning is not the ruling's + // subject, and moving it would be a behaviour change nobody asked for. + expect(body.events[0]!.occurredAt).toBe(''); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index a5b81c680e..456a24e53f 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -1698,16 +1698,17 @@ function compareAuditInstants(a: unknown, b: unknown): number { * * ⚠️ Deliberately NOT the `canonicalIsoInstant` spelling next door in * `sys-metadata-repository.ts` / `database-loader.ts` (#14037's sibling - * sites): that spelling reaches `value.toISOString()` for ANY `Date`, which - * raises `RangeError: Invalid time value` on an Invalid `Date` — measured - * reachable on BOTH live dialects (a MySQL zero datetime; any Postgres year - * in 275760..294276) and the open subject of #14078, which #13973 is - * blocked on. Whether the shared spelling should throw there (option A) or - * fall back to a rendering (option B) is a maintainer call across four - * packages, so this repair imports NEITHER answer into a new call site: an - * Invalid `Date` is returned unchanged, exactly as the raw assignment - * passed it through today. When #14078 rules, this helper collapses into - * the shared spelling. + * sites). That difference used to be exactly one input shape — the Invalid + * `Date` on which that spelling raised `RangeError: Invalid time value`, + * measured reachable on BOTH live dialects (a MySQL zero datetime; any + * Postgres year in 275760..294276). #14078 has since RULED it (option B, + * 2026-09-02): that arm is now total and answers `undefined` for the shape. + * + * ⛔ They are still not ONE spelling, and this copy has the strongest reason + * of the three not to be collapsed — see the paragraph below on what + * `listCommits` promises its callers for a non-`Date` value. The + * consolidation is tracked as **#16422**; #14078 ruled only the five arms + * that THREW. * * ⛔ NOT a tolerant fallback (#13973's standing prohibition): it teaches no * consumer to accept an off-spec shape; it converts the one measured @@ -1721,7 +1722,9 @@ function compareAuditInstants(a: unknown, b: unknown): number { * `listCommits` are promised the RAW value back untouched when it is not a * valid `Date` — an absent/opaque column must still reach `sort`'s fallback * branch and any in-process reader exactly as before. Consolidating the - * family's near-identical copies is #14078's call, not this card's. + * family's near-identical copies was expected to be #14078's call; that + * ruling covered only the five arms that threw, so the consolidation is + * tracked separately as #16422. */ function isoFromValidDate(value: unknown): unknown { if (value instanceof Date && !Number.isNaN(value.getTime())) return value.toISOString(); @@ -8625,10 +8628,30 @@ export class ObjectStackProtocolImplementation implements }); const events = (Array.isArray(rows) ? rows : []).map((r: any) => ({ id: r.id, + // [#14078] The `Date` arm is TOTAL. Ruled B by the maintainer + // (2026-09-02): every copy of this shared canonical-ISO + // spelling guards on `Number.isNaN(value.getTime())`, all five + // arms in ONE change, because a guard on some arms and not + // others re-opens the drift the single spelling closed. + // Reachability is MEASURED (#14409, `3ecb7dc1a`): mysql2 3.23.1 + // hands back a constant literally named `INVALID_DATE` for a + // zero `DATETIME`, and postgres-date 1.0.7 builds + // `new Date(NaN)` for every year in 275760..294276, which + // Postgres itself stores. Unguarded, `toISOString()` raises + // `RangeError: Invalid time value` and + // `GET /api/v1/meta/:type/:name/audit` answers 500 — Studio's + // 审计日志 tab blank, on a row the error does not name. + // The terminal value here is the VISIBLE TEXT `"Invalid Date"`: + // the guard fails, the value falls into the `String(...)` arm + // already below (which renders exactly that), and + // `AuditMetaItemResponseSchema.events[].occurredAt` is a + // REQUIRED plain `z.string()` an operator reads — the ruling's + // "required and an operator reads it". ⛔ Not `''`: a silent + // blank hides the producer's bug. occurredAt: typeof r.occurred_at === 'string' ? r.occurred_at - : r.occurred_at instanceof Date + : r.occurred_at instanceof Date && !Number.isNaN(r.occurred_at.getTime()) ? r.occurred_at.toISOString() : String(r.occurred_at ?? ''), actor: String(r.actor ?? 'system'), diff --git a/packages/metadata-protocol/src/sys-metadata-repository-14037-event-ts-canonicalisation.test.ts b/packages/metadata-protocol/src/sys-metadata-repository-14037-event-ts-canonicalisation.test.ts index 6bd0121c3f..70a5247749 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository-14037-event-ts-canonicalisation.test.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository-14037-event-ts-canonicalisation.test.ts @@ -44,12 +44,14 @@ * hand-rolled regex standing in for it. * * §C is the #14078 NEUTRALITY pin: an Invalid `Date` must reach the consumer - * UNCHANGED, exactly as this cast passes it through today. The shared - * `canonicalIsoInstant` spelling in this same file would instead raise - * `RangeError: Invalid time value` there — measured reachable on both live - * dialects — and whether it should is the open subject of #14078, which - * #13973 is blocked on. This card imports neither answer, and §C goes red the - * moment someone swaps the contested spelling in. + * UNCHANGED, exactly as this cast passes it through today. #14078 has since + * ruled (option B, 2026-09-02) and `canonicalIsoInstant` in this same file is + * now TOTAL — it answers `undefined` for that shape rather than raising + * `RangeError: Invalid time value`. The two helpers still differ across the + * REST of the input domain, so §C keeps its job unchanged: it goes red the + * moment someone swaps the other spelling into this site, which is now the + * separately-tracked consolidation decision #16422 rather than an open + * ruling. */ import { describe, it, expect, beforeEach } from 'vitest'; diff --git a/packages/metadata-protocol/src/sys-metadata-repository-14078-invalid-date-total-arm.test.ts b/packages/metadata-protocol/src/sys-metadata-repository-14078-invalid-date-total-arm.test.ts new file mode 100644 index 0000000000..a6625e5708 --- /dev/null +++ b/packages/metadata-protocol/src/sys-metadata-repository-14078-invalid-date-total-arm.test.ts @@ -0,0 +1,193 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14078] `canonicalIsoInstant`'s `Date` arm is TOTAL — an Invalid `Date` + * leaves as `undefined`, never as a `RangeError` at the serialisation seam. + * + * ## The defect + * + * The arm was `if (value instanceof Date) return value.toISOString();`, and + * `toISOString()` raises `RangeError: Invalid time value` for the one `Date` + * whose time value is `NaN`. The spelling it replaced — `String(value)` — + * served the text `"Invalid Date"` for the same input, so the repair traded a + * visibly-wrong field for an uncaught exception on a READ path: a 500 the + * operator cannot trace to a row. + * + * ## Reachability is measured, not argued + * + * PR #14409 (landed `3ecb7dc1a`) drove both live client libraries: mysql2 + * 3.23.1 returns a module constant literally named `INVALID_DATE` for a zero + * `DATETIME`, and postgres-date 1.0.7 builds `new Date(NaN)` for every year in + * 275760..294276 — a range Postgres itself stores. The shape is not + * hypothetical, which is why the maintainer ruled option B (2026-09-02) with + * the guard on all five arms of the shared spelling at once. + * + * ## What is pinned here, and why `undefined` is the terminal value at THIS arm + * + * The ruling sets the terminal value **per call site**: the visible text + * `"Invalid Date"` where the field is required and an operator reads it, + * `undefined` where the field is optional and the caller already carries a + * `?? default` chain. Both call sites of this copy are the second case — + * `getByHash` ends in `?? new Date(0).toISOString()` and `rowToItem` in + * `?? new Date().toISOString()`, the branch an absent column takes today — so + * `undefined` hands the bad row to the caller's own fallback and + * `MetadataItem.authoredAt` stays a parseable instant. Feeding the literal + * text here would instead move the failure downstream: `authoredAt` is read by + * machines, and its one in-repo forwarding lands in a `z.string().datetime()` + * field that the text fails outright. + * + * ## Why the reverse check is in every case + * + * A pin that only asserts "no throw" would stay green against a fixture that + * silently degraded to a string. Each case therefore proves the planted value + * really is a `Date` with a `NaN` time value AND evaluates the OLD spelling on + * that same object (`value.toISOString()`), asserting it raises `RangeError`. + * That is the removed guard reproduced in-place: if the input ever stopped + * being the contested shape, the reverse check goes red first. + * + * §C is the discrimination limb: a valid `Date` is still canonicalised + * byte-exactly and an already-canonical string is still a fixed point, so the + * guard cannot pass by having disabled the arm it guards. + */ + +import { describe, it, expect } from 'vitest'; +import { assertEngineFindOnePredicate, MetadataItemSchema } from '@objectstack/metadata-core'; +import { SysMetadataRepository } from './sys-metadata-repository.js'; + +/** Canonical ISO-8601 UTC with milliseconds — what the declared type promises. */ +const ISO_Z = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/; + +/** A checksum shaped as `MetadataItemSchema.hash` demands. */ +const HASH = `sha256:${'a'.repeat(64)}`; + +/** Non-zero milliseconds: a truncating regression stays observable. */ +const PG_INSTANT = new Date('2026-03-04T05:06:07.089Z'); +const SQLITE_TEXT = '2026-03-04T05:06:07.089Z'; + +/** + * The one shape both live drivers were measured to hand back. Built here, per + * case, so no case can share an object with another. + */ +function invalidDate(): Date { + return new Date(NaN); +} + +/** + * The removed guard, reproduced: the OLD arm's expression on the very object + * the case plants. Red here means the fixture is no longer the contested shape + * and every assertion below would be vacuous. + */ +function assertOldSpellingWouldThrow(value: Date): void { + expect(value, 'fixture degraded — not a Date').toBeInstanceOf(Date); + expect(Number.isNaN(value.getTime()), 'fixture is a VALID Date — case is vacuous').toBe(true); + expect(() => value.toISOString()).toThrow(RangeError); +} + +/** + * Minimal engine double: `findOne` only, which is every verb `get` and + * `getByHash` reach. It opens on the producer's own refusal predicate, so a + * query a real server would refuse cannot pass here either. + */ +function makeEngine(row: Record | null) { + return { + async findOne(table: string, opts: { where: Record }) { + assertEngineFindOnePredicate(table, opts); + return row; + }, + }; +} + +function makeRepo(row: Record | null) { + return new SysMetadataRepository({ + engine: makeEngine(row) as never, + organizationId: 'org_alpha', + orgLabel: 'org_alpha', + }); +} + +const REF = { org: 'org_alpha', type: 'view' as const, name: 'case_grid' }; +const BODY = { name: 'case_grid', label: 'Cases', object: 'case', columns: [{ field: 'name' }] }; + +function overlayRow(stamp: unknown): Record { + return { + id: 'r_1', type: 'view', name: 'case_grid', organization_id: 'org_alpha', + state: 'active', metadata: BODY, checksum: HASH, updated_by: 'usr_1', + updated_at: stamp, created_at: stamp, + }; +} + +function historyRow(stamp: unknown): Record { + return { + id: 'h_1', type: 'view', name: 'case_grid', organization_id: 'org_alpha', + metadata: BODY, checksum: HASH, previous_checksum: null, + recorded_by: 'usr_1', recorded_at: stamp, event_seq: 3, + }; +} + +describe('[#14078] §A rowToItem — an Invalid Date reaches the caller fallback, not a RangeError', () => { + it('serves the `?? new Date().toISOString()` default instead of throwing', async () => { + const bad = invalidDate(); + assertOldSpellingWouldThrow(bad); + + const before = Date.now(); + const item = await makeRepo(overlayRow(bad)).get(REF); + const after = Date.now(); + + expect(item).not.toBeNull(); + // The caller's own fallback fired — a real instant, stamped now. + expect(item!.authoredAt).toMatch(ISO_Z); + const stamped = Date.parse(item!.authoredAt); + expect(stamped).toBeGreaterThanOrEqual(before); + expect(stamped).toBeLessThanOrEqual(after); + + // ⛔ The two answers the ruling forbids at THIS arm: the visible text + // (which a `z.string().datetime()` reader downstream refuses) and the + // silent blank. + expect(item!.authoredAt).not.toBe('Invalid Date'); + expect(item!.authoredAt).not.toBe(''); + + // The declared contract, evaluated on a driver-shaped input. + const parsed = MetadataItemSchema.safeParse(item); + expect(parsed.success, JSON.stringify((parsed as never as { error?: { issues: unknown } }).error?.issues)).toBe(true); + }); +}); + +describe('[#14078] §B getByHash — the same arm, the same answer', () => { + it('falls back to the epoch default instead of throwing', async () => { + const bad = invalidDate(); + assertOldSpellingWouldThrow(bad); + + const item = await makeRepo(historyRow(bad)).getByHash(REF, HASH); + + expect(item).not.toBeNull(); + // `getByHash`'s own chain, unchanged by this card. + expect(item!.authoredAt).toBe(new Date(0).toISOString()); + expect(MetadataItemSchema.safeParse(item).success).toBe(true); + }); +}); + +describe('[#14078] §C the guard discriminates — the arm it guards still works', () => { + it('canonicalises a VALID Date byte-exactly', async () => { + const item = await makeRepo(overlayRow(PG_INSTANT)).get(REF); + expect(item!.authoredAt).toBe(PG_INSTANT.toISOString()); + expect(item!.authoredAt).toMatch(ISO_Z); + }); + + it('leaves an already-canonical SQLite string byte-identical', async () => { + const item = await makeRepo(overlayRow(SQLITE_TEXT)).get(REF); + expect(item!.authoredAt).toBe(SQLITE_TEXT); + }); + + it('still reads an absent column as absent, not as an Invalid Date', async () => { + const row = overlayRow(PG_INSTANT); + delete row.updated_at; delete row.created_at; + + const before = Date.now(); + const item = await makeRepo(row).get(REF); + + // Same branch the Invalid `Date` now takes — which is the point of + // choosing `undefined`: one meaning, "this row carries no usable instant". + expect(item!.authoredAt).toMatch(ISO_Z); + expect(Date.parse(item!.authoredAt)).toBeGreaterThanOrEqual(before); + }); +}); diff --git a/packages/metadata-protocol/src/sys-metadata-repository.ts b/packages/metadata-protocol/src/sys-metadata-repository.ts index a36742097b..49df62f1d2 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.ts @@ -114,10 +114,41 @@ import { isWritablePackage } from './package-writability.js'; * * Absent column -> `undefined`, so each caller's existing `?? ` chain * keeps exactly its current meaning. + * + * ## [#14078] The `Date` arm is TOTAL — an Invalid `Date` yields `undefined` + * + * Ruled **B** by the maintainer (2026-09-02): every copy of this spelling + * guards on `Number.isNaN(value.getTime())`, all five arms in ONE change, + * because a guard on some arms and not others re-opens the drift the single + * spelling closed. + * + * Reachability is MEASURED, not assumed (#14409, landed `3ecb7dc1a`): mysql2 + * 3.23.1 returns a module constant literally named `INVALID_DATE` for a zero + * `DATETIME`, and postgres-date 1.0.7 builds `new Date(NaN)` for every year in + * 275760..294276 — years Postgres itself stores. Unguarded, + * `value.toISOString()` raises `RangeError: Invalid time value`, so a read + * endpoint answers **500** on a row the operator cannot identify from the + * error; the spelling this replaced (`String(value)`) served the visible text + * `"Invalid Date"` instead. + * + * The terminal value is chosen **per call site**, and this one's is + * `undefined`: both callers (`getByHash` and `rowToItem`) already end in + * `?? new Date(...).toISOString()`, the branch an absent column takes today. + * The ruling assigns `undefined` exactly where "the field is optional and the + * caller already carries a `?? default` chain". ⛔ NOT the visible text + * `"Invalid Date"` — the fields fed from here are read by machines + * (`MetadataItem.authoredAt`, whose one in-repo forwarding lands in a + * `z.string().datetime()` field), so that text would move the failure to a zod + * refusal at the consumer instead of removing it. ⛔ And NOT a blanket `''`: a silent blank is the shape that + * hides the producer's bug. */ function canonicalIsoInstant(value: unknown): string | undefined { if (value === null || value === undefined) return undefined; - if (value instanceof Date) return value.toISOString(); + // [#14078] Total `Date` arm: an Invalid `Date` is the ONE `Date` shape + // `toISOString()` refuses, and it is reachable from two live drivers. It + // leaves as `undefined` so the caller's `?? ` chain — the same one + // an absent column takes — keeps exactly its current meaning. + if (value instanceof Date) return Number.isNaN(value.getTime()) ? undefined : value.toISOString(); if (typeof value === 'string') return value; return String(value); } @@ -139,17 +170,24 @@ function canonicalIsoInstant(value: unknown): string | undefined { * — `MetadataManager.applyRepoEvent`, which forwards it to * `MetadataWatchEvent.timestamp` — is declared `z.string().datetime()`. * - * ⚠️ Deliberately NOT {@link canonicalIsoInstant} above, and the difference is - * exactly one input shape. That spelling reaches `value.toISOString()` for ANY - * `Date`, which raises `RangeError: Invalid time value` on an Invalid `Date` - * — measured reachable on BOTH live dialects (a MySQL zero datetime; any - * Postgres year in 275760..294276) and the open subject of #14078, which - * #13973 is blocked on. Whether the shared spelling should throw there - * (option A) or fall back to a rendering (option B) is a maintainer call over - * four packages, so this repair imports NEITHER answer into a new call site: - * an Invalid `Date` is returned unchanged, exactly as this cast passes it - * through today. When #14078 rules, this helper collapses into the shared - * spelling. + * ⚠️ Deliberately NOT {@link canonicalIsoInstant} above. That difference used + * to be exactly one input shape — the Invalid `Date` on which that spelling + * raised `RangeError: Invalid time value`, measured reachable on BOTH live + * dialects (a MySQL zero datetime; any Postgres year in 275760..294276). + * #14078 has since RULED it (option B, 2026-09-02): that arm is now total and + * answers `undefined` for the shape, so the two agree on it. + * + * ⛔ They are still not ONE spelling, which is why #14078 did not collapse + * this helper into it. `canonicalIsoInstant` returns `string | undefined` and + * rewrites the whole domain (nullish -> `undefined`; anything neither `Date` + * nor string -> `String(value)`), while this one returns `unknown` and hands + * every non-valid-`Date` shape back UNTOUCHED. The consolidation is its own + * decision — **#16422** — because it moves six call sites for `null`, for a + * `number` and for an opaque column, one of which (`MetadataHistoryRecord + * .recordedAt`, a REQUIRED `z.string().datetime()`) has no terminal value + * either half of the #14078 ruling supplies. §C of + * `sys-metadata-repository-14037-event-ts-canonicalisation.test.ts` pins the + * behaviour this paragraph describes. * * ⛔ NOT a tolerant fallback (#13973's standing prohibition): it teaches no * consumer to accept an off-spec shape; it converts one measured producer diff --git a/packages/metadata/src/loaders/database-loader-14037-adapter-boundary-iso.test.ts b/packages/metadata/src/loaders/database-loader-14037-adapter-boundary-iso.test.ts index 3ce870ca1f..356dfb3dd6 100644 --- a/packages/metadata/src/loaders/database-loader-14037-adapter-boundary-iso.test.ts +++ b/packages/metadata/src/loaders/database-loader-14037-adapter-boundary-iso.test.ts @@ -51,11 +51,13 @@ * * §D is the #14078 NEUTRALITY pin and is load-bearing for this card's scope: * an Invalid `Date` must reach the consumer UNCHANGED, exactly as these casts - * pass it through today. The shared `canonicalIsoInstant` spelling would - * instead raise `RangeError: Invalid time value` there — measured reachable on - * both live dialects — and whether it should is the open subject of #14078, - * which #13973 is blocked on. This card imports neither answer, and §D goes - * red the moment someone swaps the contested spelling in. + * pass it through today. #14078 has since ruled (option B, 2026-09-02) and the + * shared `canonicalIsoInstant` spelling is now TOTAL — it answers `undefined` + * for that shape rather than raising `RangeError: Invalid time value`. The two + * helpers still differ across the REST of the input domain, so §D keeps its + * job unchanged: it goes red the moment someone swaps the other spelling in, + * which is now the separately-tracked consolidation decision #16422 rather + * than an open ruling. */ import { describe, it, expect, beforeEach } from 'vitest'; diff --git a/packages/metadata/src/loaders/database-loader-14078-invalid-date-total-arm.test.ts b/packages/metadata/src/loaders/database-loader-14078-invalid-date-total-arm.test.ts new file mode 100644 index 0000000000..44a346e050 --- /dev/null +++ b/packages/metadata/src/loaders/database-loader-14078-invalid-date-total-arm.test.ts @@ -0,0 +1,193 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14078] `canonicalIsoInstant`'s `Date` arm is TOTAL — an Invalid `Date` + * leaves as `undefined`, so `stat()`'s own `?? new Date().toISOString()` chain + * publishes a parseable `MetadataStats.mtime` instead of the read raising + * `RangeError: Invalid time value`. + * + * ## The defect + * + * The arm was `if (value instanceof Date) return value.toISOString();`. For + * the one `Date` whose time value is `NaN` that call throws, and `stat()` is a + * hot read path (REST `/meta/*`, ObjectQL plan resolution, runtime overlay + * merges) — so one legacy row answered 500 where the spelling this replaced, + * `String(value)`, had served the visible text `"Invalid Date"`. + * + * ## Reachability is measured, not argued + * + * PR #14409 (landed `3ecb7dc1a`): mysql2 3.23.1 returns a module constant + * literally named `INVALID_DATE` for a zero `DATETIME`; postgres-date 1.0.7 + * builds `new Date(NaN)` for every year in 275760..294276, which Postgres + * itself stores. Ruled option B by the maintainer on 2026-09-02, on all five + * arms of the shared spelling at once. + * + * ## Why `undefined` is the terminal value at THIS arm + * + * The ruling sets it per call site. `stat()` — the only caller — already ends + * in `?? new Date().toISOString()`, the branch an absent column takes today, + * which is the ruling's "optional, and the caller already carries a + * `?? default` chain". And `MetadataStats.mtime` is declared + * `z.string().datetime()` in `packages/spec/src/system/metadata-persistence.zod.ts`: + * feeding it the literal text `"Invalid Date"` would not produce a visible + * cell, it would produce a zod refusal at the consumer — the same 500 moved + * one layer out. ⛔ And not `''`, the silent blank the ruling forbids. + * + * ## The composition this file also pins + * + * `stat()` reads `record.updatedAt` out of `rowToRecord`, which canonicalises + * through the SEPARATE `isoFromValidDate` helper (#14037) — and that helper + * hands an Invalid `Date` through UNCHANGED by design. So the bad value + * reaches `canonicalIsoInstant` as a `Date`, not as text, and this arm is the + * one that has to be total. §C states that as an assertion so the two helpers + * cannot drift apart unnoticed. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { assertEngineFindOnePredicate } from '@objectstack/metadata-core'; +import { MetadataStatsSchema } from '@objectstack/spec/system'; +import type { IDataEngine } from '@objectstack/spec/contracts'; +import { DatabaseLoader } from './database-loader.js'; + +type Row = Record; + +/** Canonical ISO-8601 UTC with milliseconds — what `mtime` is declared as. */ +const ISO_Z = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/; + +/** Non-zero milliseconds, so a truncating regression stays observable. */ +const PG_INSTANT = new Date('2026-03-04T05:06:07.089Z'); +const SQLITE_TEXT = '2026-03-04T05:06:07.089Z'; + +/** + * The removed guard, reproduced: the OLD arm's expression on the very object + * the case plants. Red here means the fixture is no longer the contested shape + * and every assertion below would be vacuous. + */ +function assertOldSpellingWouldThrow(value: Date): void { + expect(value, 'fixture degraded — not a Date').toBeInstanceOf(Date); + expect(Number.isNaN(value.getTime()), 'fixture is a VALID Date — case is vacuous').toBe(true); + expect(() => value.toISOString()).toThrow(RangeError); +} + +/** + * Minimal read-only engine double — the same shape the #14037 sibling in this + * directory uses. Stores and returns exactly what it is handed, so a `Date` + * planted in a row survives to the read door the way a live driver's would. + */ +function makeReadEngine(tables: Record) { + const matches = (r: Row, where: Record): boolean => + Object.entries(where ?? {}).every(([k, v]) => { + if (k.startsWith('$')) throw new Error(`engine double: unsupported operator ${k}`); + return v === undefined || r[k] === v; + }); + const rowsOf = (table: string): Row[] => tables[table] ?? []; + return { + async find(table: string, opts: { where: Record; limit?: number }) { + const matched = rowsOf(table).filter((r) => matches(r, opts?.where)); + return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched; + }, + async findOne(table: string, opts: { where: Record }) { + assertEngineFindOnePredicate(table, opts); + return rowsOf(table).find((r) => matches(r, opts.where)) ?? null; + }, + async count(table: string, opts: { where: Record }) { + return rowsOf(table).filter((r) => matches(r, opts?.where)).length; + }, + } as unknown as IDataEngine; +} + +function metadataRow(overrides: Row = {}): Row { + return { + id: 'meta_1', name: 'case_grid', type: 'view', namespace: 'default', + managed_by: 'platform', scope: 'platform', state: 'active', version: 3, + metadata: { name: 'case_grid', object: 'case' }, + checksum: 'sha256-abc', source: 'database', + created_by: 'usr_1', updated_by: 'usr_1', + ...overrides, + }; +} + +describe('[#14078] DatabaseLoader.stat — an Invalid Date yields the caller fallback, not a RangeError', () => { + let tables: Record; + const loaderFor = (row: Row) => { + tables = { sys_metadata: [row], sys_metadata_history: [] }; + return new DatabaseLoader({ engine: makeReadEngine(tables) }); + }; + + beforeEach(() => { + tables = { sys_metadata: [], sys_metadata_history: [] }; + }); + + describe('§A the contested shape', () => { + it('publishes a parseable mtime instead of throwing', async () => { + const bad = new Date(NaN); + assertOldSpellingWouldThrow(bad); + + const before = Date.now(); + const stats = await loaderFor(metadataRow({ updated_at: bad, created_at: bad })).stat('view', 'case_grid'); + const after = Date.now(); + + expect(stats).not.toBeNull(); + expect(stats!.mtime).toMatch(ISO_Z); + const stamped = Date.parse(stats!.mtime!); + expect(stamped).toBeGreaterThanOrEqual(before); + expect(stamped).toBeLessThanOrEqual(after); + + // ⛔ The two answers the ruling forbids at THIS arm: the visible text + // (which `z.string().datetime()` refuses downstream) and the blank. + expect(stats!.mtime).not.toBe('Invalid Date'); + expect(stats!.mtime).not.toBe(''); + + // The declared contract — `mtime` is `z.string().datetime()`, so this + // limb is exactly what rules the visible text out at this call site. + const parsed = MetadataStatsSchema.safeParse(stats); + expect(parsed.success, JSON.stringify((parsed as { error?: { issues: unknown } }).error?.issues)).toBe(true); + }); + + it('falls back for updated_at alone, without letting `??` mistake it for absent', async () => { + const bad = new Date(NaN); + assertOldSpellingWouldThrow(bad); + + // `canonicalIsoInstant(record.updatedAt ?? record.createdAt)` — an + // Invalid `Date` is NOT nullish, so `??` does not fall through to + // `created_at`. The guard, not the `??`, is what makes this total. + const stats = await loaderFor(metadataRow({ updated_at: bad, created_at: PG_INSTANT })).stat('view', 'case_grid'); + + expect(stats!.mtime).not.toBe(PG_INSTANT.toISOString()); + expect(stats!.mtime).toMatch(ISO_Z); + expect(MetadataStatsSchema.safeParse(stats).success).toBe(true); + }); + }); + + describe('§B the guard discriminates — the arm it guards still works', () => { + it('canonicalises a VALID Date byte-exactly', async () => { + const stats = await loaderFor(metadataRow({ updated_at: PG_INSTANT })).stat('view', 'case_grid'); + expect(stats!.mtime).toBe(PG_INSTANT.toISOString()); + }); + + it('leaves an already-canonical SQLite string byte-identical', async () => { + const stats = await loaderFor(metadataRow({ updated_at: SQLITE_TEXT })).stat('view', 'case_grid'); + expect(stats!.mtime).toBe(SQLITE_TEXT); + }); + }); + + describe('§C the composition with #14037 `isoFromValidDate`, stated as an assertion', () => { + it('hands the Invalid Date to this arm as a Date — the other helper does NOT convert it', async () => { + const bad = new Date(NaN); + const loader = loaderFor(metadataRow({ updated_at: bad })); + + // `rowToRecord` is the step before `stat`'s own; it routes through the + // separate `isoFromValidDate` helper, which passes this shape through + // untouched. If that ever changes, the arm under test stops being the + // one that has to be total and this file should be re-read. + const record = (loader as unknown as { rowToRecord(r: Row): Record }) + .rowToRecord(metadataRow({ updated_at: bad })); + expect(record.updatedAt).toBe(bad); + expect(record.updatedAt).toBeInstanceOf(Date); + + // …and the total arm is what turns that into a contract-satisfying stat. + const stats = await loader.stat('view', 'case_grid'); + expect(MetadataStatsSchema.safeParse(stats).success).toBe(true); + }); + }); +}); diff --git a/packages/metadata/src/loaders/database-loader.ts b/packages/metadata/src/loaders/database-loader.ts index 5fd4be41f2..ab705f3c85 100644 --- a/packages/metadata/src/loaders/database-loader.ts +++ b/packages/metadata/src/loaders/database-loader.ts @@ -57,10 +57,41 @@ import { migrateProjectIdToEnvironmentId } from '../migrations/migrate-project-i * producer — the same shape the sibling adapters in * `@objectstack/metadata-protocol` apply. Absent column -> `undefined`, so the * caller's existing `?? ` chain keeps its current meaning. + * + * ## [#14078] The `Date` arm is TOTAL — an Invalid `Date` yields `undefined` + * + * Ruled **B** by the maintainer (2026-09-02): every copy of this spelling + * guards on `Number.isNaN(value.getTime())`, all five arms in ONE change, + * because a guard on some arms and not others re-opens the drift the single + * spelling closed. + * + * Reachability is MEASURED, not assumed (#14409, landed `3ecb7dc1a`): mysql2 + * 3.23.1 returns a module constant literally named `INVALID_DATE` for a zero + * `DATETIME`, and postgres-date 1.0.7 builds `new Date(NaN)` for every year in + * 275760..294276 — years Postgres itself stores. Unguarded, + * `value.toISOString()` raises `RangeError: Invalid time value`, so a read + * endpoint answers **500** on a row the operator cannot identify from the + * error; the spelling this replaced (`String(value)`) served the visible text + * `"Invalid Date"` instead. + * + * The terminal value is chosen **per call site**, and this one's is + * `undefined`: the one caller (`stat`) already ends in + * `?? new Date().toISOString()`, the branch an absent column takes today. The + * ruling assigns `undefined` exactly where "the field is optional and the + * caller already carries a `?? default` chain". ⛔ NOT the visible text + * `"Invalid Date"` — the fields fed from here are read by machines + * (`MetadataStats.mtime` is declared `z.string().datetime().optional()` in + * `packages/spec`), so that text would move the failure to a zod refusal at + * the consumer instead of removing it. ⛔ And NOT a blanket `''`: a silent blank is the shape that + * hides the producer's bug. */ function canonicalIsoInstant(value: unknown): string | undefined { if (value === null || value === undefined) return undefined; - if (value instanceof Date) return value.toISOString(); + // [#14078] Total `Date` arm: an Invalid `Date` is the ONE `Date` shape + // `toISOString()` refuses, and it is reachable from two live drivers. It + // leaves as `undefined` so the caller's `?? ` chain — the same one + // an absent column takes — keeps exactly its current meaning. + if (value instanceof Date) return Number.isNaN(value.getTime()) ? undefined : value.toISOString(); if (typeof value === 'string') return value; return String(value); } @@ -85,17 +116,24 @@ function canonicalIsoInstant(value: unknown): string | undefined { * `Date` fails outright. The cast is an assertion about a driver row, never a * measurement of one, which is why tsc reports nothing. * - * ⚠️ Deliberately NOT {@link canonicalIsoInstant} above, and the difference is - * exactly one input shape. That spelling reaches `value.toISOString()` for ANY - * `Date`, which raises `RangeError: Invalid time value` on an Invalid `Date` - * — measured reachable on BOTH live dialects (a MySQL zero datetime; any - * Postgres year in 275760..294276) and the open subject of #14078, which - * #13973 is blocked on. Whether the shared spelling should throw there - * (option A) or fall back to a rendering (option B) is a maintainer call over - * four packages, so this repair imports NEITHER answer into five new call - * sites: an Invalid `Date` is returned unchanged, exactly as these casts pass - * it through today. When #14078 rules, this helper collapses into the shared - * spelling. + * ⚠️ Deliberately NOT {@link canonicalIsoInstant} above. That difference used + * to be exactly one input shape — the Invalid `Date` on which that spelling + * raised `RangeError: Invalid time value`, measured reachable on BOTH live + * dialects (a MySQL zero datetime; any Postgres year in 275760..294276). + * #14078 has since RULED it (option B, 2026-09-02): that arm is now total and + * answers `undefined` for the shape, so the two agree on it. + * + * ⛔ They are still not ONE spelling, which is why #14078 did not collapse + * this helper into it. `canonicalIsoInstant` returns `string | undefined` and + * rewrites the whole domain (nullish -> `undefined`; anything neither `Date` + * nor string -> `String(value)`), while this one returns `unknown` and hands + * every non-valid-`Date` shape back UNTOUCHED. The consolidation is its own + * decision — **#16422** — because it moves six call sites for `null`, for a + * `number` and for an opaque column, one of which (`MetadataHistoryRecord + * .recordedAt`, a REQUIRED `z.string().datetime()` fed twice from here) has no + * terminal value either half of the #14078 ruling supplies. §D of + * `database-loader-14037-adapter-boundary-iso.test.ts` pins the behaviour this + * paragraph describes. * * ⛔ NOT a tolerant fallback (#13973's standing prohibition): it teaches no * consumer to accept an off-spec shape; it converts one measured producer diff --git a/packages/rest/src/rest-14078-invalid-date-total-arm.test.ts b/packages/rest/src/rest-14078-invalid-date-total-arm.test.ts new file mode 100644 index 0000000000..dc8b2d45eb --- /dev/null +++ b/packages/rest/src/rest-14078-invalid-date-total-arm.test.ts @@ -0,0 +1,243 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14078] The two `@objectstack/rest` arms of the shared canonical-ISO + * spelling are TOTAL — an Invalid `Date` is served as the visible text + * `"Invalid Date"`, never as a `RangeError` at the serialisation seam. + * + * ## The defect + * + * `canonicalIsoStamp` (the import-job DTO) and `formatCsvCell` (the export + * writer) both reached `value.toISOString()` for ANY `Date`, and that call + * raises `RangeError: Invalid time value` for the one `Date` whose time value + * is `NaN`. Both are READ paths, so the loudness landed as a 500 the operator + * cannot trace to a row — where the spelling both repairs replaced, + * `String(value)`, had served the text `"Invalid Date"` in the cell. + * + * ## Reachability is measured, not argued + * + * PR #14409 (landed `3ecb7dc1a`) drove both live client libraries: mysql2 + * 3.23.1 returns a module constant literally named `INVALID_DATE` for a zero + * `DATETIME`, and postgres-date 1.0.7 builds `new Date(NaN)` for every year in + * 275760..294276 — a range Postgres itself stores. The maintainer ruled option + * B on 2026-09-02, on all five arms of the shared spelling at once. + * + * ## Why the terminal value here is the TEXT, not `undefined` + * + * The ruling sets it per call site: visible text where the field is required + * and an operator reads it. Both arms here are that case, and the declared + * contracts are what prove it rather than intuition — + * `ImportJobProgressSchema.createdAt` is a REQUIRED **plain** `z.string()` + * (`packages/spec/src/api/export.zod.ts`), not `z.string().datetime()`, so the + * text satisfies the contract and reaches the operator watching the job; a CSV + * cell has no schema at all and is read by a human in a spreadsheet. ⛔ Not a + * blanket `''`: a silent blank is the shape that hides the producer's bug — + * and in the CSV case it is what `JSON.stringify` would have produced anyway + * (`Date.prototype.toJSON` answers `null` for an Invalid `Date`), which is + * exactly why that arm needs its own guard rather than a fall-through. + * + * ## Both CSV paths land on the same arm + * + * With field metadata, a `datetime` cell goes through `export-format.ts`'s + * `formatDate`, whose `toDate` helper REJECTS an Invalid `Date` and returns + * the value unchanged — so the raw path and the formatted path both hand the + * `Date` to `formatCsvCell`. §B drives both. + * + * ## What makes these cases non-vacuous + * + * Every case proves its planted value is a `Date` with a `NaN` time value and + * evaluates the OLD arm's expression on that same object, asserting it raises + * `RangeError`. §C is the discrimination limb: a valid `Date` is still + * canonicalised and a canonical string is still a fixed point, so a guard that + * had simply disabled the arm it guards cannot pass. + * + * ⛔ No driver dependency: the shape under test is the one a SQLite-backed + * engine cannot produce, and that unreachability is what hid the family of + * defects this card closes. The read door is stubbed, deliberately, exactly as + * the #13994 sibling in this package argues. + */ + +import { describe, it, expect } from 'vitest'; +import { ImportJobProgressSchema, ImportJobSummarySchema } from '@objectstack/spec/api'; +import { RestServer } from './rest-server'; + +/** Canonical ISO-8601 UTC with milliseconds — what the DTO contract promises. */ +const CANONICAL_ISO = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/; + +/** The rendering the pre-repair spelling produced for this shape. */ +const INVALID_TEXT = 'Invalid Date'; + +const CREATED = '2026-08-30T10:19:25.947Z'; +const STARTED = '2026-08-30T10:20:31.001Z'; + +/** + * The removed guard, reproduced: the OLD arm's expression on the very object + * a case plants. Red here means the fixture is no longer the contested shape + * and every assertion around it would be vacuous. + */ +function assertOldSpellingWouldThrow(value: Date): void { + expect(value, 'fixture degraded — not a Date').toBeInstanceOf(Date); + expect(Number.isNaN(value.getTime()), 'fixture is a VALID Date — case is vacuous').toBe(true); + expect(() => value.toISOString()).toThrow(RangeError); +} + +function createMockServer() { + const noop = () => {}; + return { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} }; +} + +function makeRes() { + const chunks: string[] = []; + const res: any = { + write: (s: string) => { chunks.push(typeof s === 'string' ? s : String(s)); return true; }, + end: () => {}, + header: () => res, + status: (code: number) => { res._status = code; return res; }, + json: (body: any) => { res._json = body; return res; }, + }; + return { res, chunks }; +} + +/** The REAL routes, over a protocol whose read door returns exactly `rows`. */ +function boot(rows: unknown[], schema?: unknown) { + const protocol: Record = { + findData: async () => ({ records: rows }), + }; + if (schema) protocol.getMetaItem = async () => ({ item: schema }); + const rest = new RestServer(createMockServer() as any, protocol as any, { api: { requireAuth: false } } as any); + (rest as any).resolveExecCtx = async () => ({ userId: 'test-user' }); + rest.registerRoutes(); + const routes = rest.getRoutes(); + const find = (method: string, path: string) => routes.find((r: any) => r.method === method && r.path === path); + return { + progress: find('GET', '/api/v1/data/import/jobs/:jobId'), + list: find('GET', '/api/v1/data/import/jobs'), + exportRoute: find('GET', '/api/v1/data/:object/export'), + }; +} + +function importJobRow(stamp: unknown): Record { + return { + id: 'imp_14078', object_name: 'task', status: 'succeeded', + dry_run: false, write_mode: 'insert', + total_rows: 1, processed_rows: 1, + created_count: 1, updated_count: 0, skipped_count: 0, error_count: 0, + created_at: stamp, + started_at: STARTED, + }; +} + +async function callJson(route: any, req: any = {}) { + const { res } = makeRes(); + await route.handler({ params: { jobId: 'imp_14078' }, query: {}, ...req } as any, res); + return (res as any)._json; +} + +async function callCsv(route: any, rowKeys: Record) { + const { res, chunks } = makeRes(); + await route.handler({ params: { object: 'task' }, query: { format: 'csv', ...rowKeys } } as any, res); + return chunks.join(''); +} + +describe('[#14078] §A canonicalIsoStamp — the import-job DTO serves text, not a 500', () => { + it('renders `Invalid Date` for a required stamp and still satisfies the declared contract', async () => { + const bad = new Date(NaN); + assertOldSpellingWouldThrow(bad); + + const body = await callJson(boot([importJobRow(bad)]).progress); + + expect(body.createdAt).toBe(INVALID_TEXT); + // The rendering is the pre-repair spelling's own, not a literal + // invented here: `String(new Date(NaN))` is `"Invalid Date"`. + expect(body.createdAt).toBe(String(bad)); + // ⛔ The blank the ruling forbids by name. + expect(body.createdAt).not.toBe(''); + + // The contract — `createdAt` is a REQUIRED plain `z.string()`, which is + // precisely why the visible text is servable at THIS call site. + const parsed = ImportJobProgressSchema.safeParse(body); + expect(parsed.success, JSON.stringify((parsed as any).error?.issues)).toBe(true); + + // The good stamp beside it is untouched — one bad column does not take + // the row down with it. + expect(body.startedAt).toBe(STARTED); + }); + + it('serves the summary (list) DTO the same way', async () => { + const bad = new Date(NaN); + assertOldSpellingWouldThrow(bad); + + const body = await callJson(boot([importJobRow(bad)]).list); + const [job] = body.jobs; + + expect(job.createdAt).toBe(INVALID_TEXT); + expect(ImportJobSummarySchema.safeParse(job).success).toBe(true); + }); +}); + +describe('[#14078] §B formatCsvCell — the export writer emits the cell an operator reads', () => { + const TASK_SCHEMA = { name: 'task', fields: { id: { type: 'text', label: 'ID' }, due: { type: 'datetime', label: '截止' } } }; + + it('writes `Invalid Date` on the RAW path (no field metadata)', async () => { + const bad = new Date(NaN); + assertOldSpellingWouldThrow(bad); + + const csv = await callCsv(boot([{ id: '1', due: bad }]).exportRoute, {}); + + expect(csv).toContain(INVALID_TEXT); + // The whole file, not a fragment: header + one data row, and the cell + // is exactly the text — never a blank cell, never `null`. + expect(csv.split('\r\n').filter((l) => l.length > 0).at(-1)).toBe(`1,${INVALID_TEXT}`); + }); + + it('writes `Invalid Date` on the FORMATTED path too (declared `datetime` field)', async () => { + const bad = new Date(NaN); + assertOldSpellingWouldThrow(bad); + + const csv = await callCsv(boot([{ id: '1', due: bad }], TASK_SCHEMA).exportRoute, {}); + + // `formatDate` -> `toDate` rejects an Invalid `Date` and returns the + // value UNCHANGED, so this path reaches the same arm. + const lines = csv.split('\r\n').filter((l) => l.length > 0); + expect(lines[0]).toBe('ID,截止'); + expect(lines[1]).toBe(`1,${INVALID_TEXT}`); + }); + + it('⛔ is NOT the `JSON.stringify` arm — that would have written a blank the ruling forbids', async () => { + const bad = new Date(NaN); + // The arm the value would have fallen into without its own guard: + // `toJSON()` answers null for an Invalid `Date`. + expect(JSON.stringify(bad)).toBe('null'); + expect(bad.toJSON()).toBeNull(); + + const csv = await callCsv(boot([{ id: '1', due: bad }]).exportRoute, {}); + expect(csv).not.toContain('null'); + }); +}); + +describe('[#14078] §C the guards discriminate — the arms they guard still work', () => { + it('canonicalises a VALID Date in the DTO, byte-exactly', async () => { + const good = new Date(CREATED); + const body = await callJson(boot([importJobRow(good)]).progress); + + expect(body.createdAt).toBe(CREATED); + expect(body.createdAt).toMatch(CANONICAL_ISO); + expect(ImportJobProgressSchema.safeParse(body).success).toBe(true); + }); + + it('leaves an already-canonical string a fixed point in the DTO', async () => { + const body = await callJson(boot([importJobRow(CREATED)]).progress); + expect(body.createdAt).toBe(CREATED); + }); + + it('canonicalises a VALID Date in a CSV cell, byte-exactly', async () => { + const good = new Date(CREATED); + const csv = await callCsv(boot([{ id: '1', due: good }]).exportRoute, {}); + expect(csv.split('\r\n').filter((l) => l.length > 0).at(-1)).toBe(`1,${CREATED}`); + }); + + it('still writes an empty cell for an absent value — that meaning is unchanged', async () => { + const csv = await callCsv(boot([{ id: '1', due: null }]).exportRoute, {}); + expect(csv.split('\r\n').filter((l) => l.length > 0).at(-1)).toBe('1,'); + }); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 4d20c874c6..65afcc4c90 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -657,6 +657,32 @@ function importJobUndoable(row: any): boolean { * declaration is right; the emitted value was wrong. This makes the value what * the declaration already says. * + * ## [#14078] The `Date` arm is TOTAL — an Invalid `Date` renders as text + * + * Ruled **B** by the maintainer (2026-09-02): every copy of this spelling + * guards on `Number.isNaN(value.getTime())`, all five arms in ONE change, + * because a guard on some arms and not others re-opens the drift the single + * spelling closed. Reachability is MEASURED, not assumed (#14409, landed + * `3ecb7dc1a`): mysql2 3.23.1 returns a module constant literally named + * `INVALID_DATE` for a zero `DATETIME`, and postgres-date 1.0.7 builds + * `new Date(NaN)` for every year in 275760..294276 — years Postgres itself + * stores. Unguarded, `value.toISOString()` raises `RangeError: Invalid time + * value`, so `GET /api/v1/data/import/jobs/:jobId` answers **500** on a job + * row the operator cannot identify from the error. + * + * The terminal value is chosen **per call site**, and this one's is the + * VISIBLE TEXT `"Invalid Date"`, reached by letting the Invalid `Date` fall + * into the `String(value ?? '')` arm — the "rendered as before" branch this + * docblock already describes, and the spelling the #13994 repair replaced. + * Why not `undefined`, the answer the `metadata-protocol` / `metadata` copies + * take: this function returns `string` because its four call sites are the + * DTO's last step, `ImportJobProgressSchema.createdAt` is required, and all + * four fields are declared plain `z.string()` (`packages/spec/src/api/ + * export.zod.ts`) rather than `z.string().datetime()` — so the text passes the + * contract and reaches the operator watching the import job, which is the + * ruling's "required and an operator reads it". ⛔ And NOT a blanket `''`: a + * silent blank is the shape that hides the producer's bug. + * * Same three branches as the two landed normalisers in * `@objectstack/metadata-protocol` — `auditMetaItem`'s `occurredAt` in * `protocol.ts` and `canonicalIsoInstant` in `sys-metadata-repository.ts` @@ -670,7 +696,12 @@ function importJobUndoable(row: any): boolean { */ function canonicalIsoStamp(value: unknown): string { if (typeof value === 'string') return value; - if (value instanceof Date) return value.toISOString(); + // [#14078] The `Date` arm is TOTAL: an Invalid `Date` fails the guard and + // falls into the "rendered as before" arm below, which is `String(value)` + // — exactly the visible text `"Invalid Date"` this repair's predecessor + // served. See the docblock's Invalid-`Date` paragraph for why the text and + // not `undefined` here. + if (value instanceof Date && !Number.isNaN(value.getTime())) return value.toISOString(); return String(value ?? ''); } @@ -726,7 +757,15 @@ function formatCsvCell(value: any): string { let s: string; if (typeof value === 'string') s = value; else if (typeof value === 'number' || typeof value === 'boolean' || typeof value === 'bigint') s = String(value); - else if (value instanceof Date) s = value.toISOString(); + // [#14078] Total `Date` arm. An Invalid `Date` renders as `String(value)`, + // which is exactly the visible text `Invalid Date` — the cell an operator + // reads and can report. ⛔ Not the `JSON.stringify` arm below: `toJSON()` + // answers `null` for an Invalid `Date`, so the bad row would arrive as the + // silent blank the ruling forbids. Both CSV paths land here: the formatted + // path's `formatDate` returns the value UNCHANGED when `toDate` rejects it + // (`export-format.ts`), so this arm is the terminal for the raw path and + // the field-metadata path alike. + else if (value instanceof Date) s = Number.isNaN(value.getTime()) ? String(value) : value.toISOString(); else { try { s = JSON.stringify(value); } catch { s = String(value); } } if (/[",\r\n]/.test(s)) { return `"${s.replace(/"/g, '""')}"`; diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 69ce92aa70..e4c89c3fc4 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -1726,6 +1726,11 @@ "verb": "findOne", "pinned": 1 }, + { + "file": "packages/metadata/src/loaders/database-loader-14078-invalid-date-total-arm.test.ts", + "verb": "findOne", + "pinned": 1 + }, { "file": "packages/metadata/src/loaders/database-loader-update-id-fold-wins.test.ts", "verb": "update",