fix(metadata-core,metadata-fs): hash the serialized form, so put().version identifies the bytes actually stored (#7856)

.changeset/hashspec-serialized-form.md

@@ -0,0 +1,65 @@
+---
+"@objectstack/metadata-core": minor
+"@objectstack/metadata-fs": patch
+---
+
+fix(metadata-core,metadata-fs): hash the serialized form, so `put().version` identifies the bytes actually stored (#7856)
+
+`hashSpec` canonicalised a `Date` to `{}`, because `canonicalize` walked a
+value's own enumerable keys and a `Date` has none. `JSON.stringify` — what every
+repository actually writes — turns the same `Date` into an ISO string. So the
+hash of the in-memory spec and the hash of the bytes on disk were **different
+hashes for the same item**, and the version handed back to a caller did not
+identify what had been stored.
+
+Measured on `main`, one spec carrying one `Date`:
+
+```
+canonicalize(in-memory) : {"createdAt":{},"label":"Home"}
+JSON.stringify (bytes) : {"label":"Home","createdAt":"2024-01-01T00:00:00.000Z"}
+```
+
+`canonicalize` now honours `toJSON` exactly as `JSON.stringify` does —
+consulted once per position, its result serialised as-is and never
+re-consulted — which makes a new guarantee true by construction:
+
+```
+canonicalize(x) === canonicalize(JSON.parse(JSON.stringify(x)))
+```
+
+**Both repository implementations were wrong, in different places**, which is
+why the fix is one function rather than two patches. `FileSystemRepository`
+broke `put().version === get().hash`: it hashed the spec it was handed, wrote
+`JSON.stringify` of it, and re-hashed the parse on the way back out.
+`InMemoryRepository` broke the repository contract's invariant 4
+(`item.hash === hashSpec(item.body)`): it stores `body` already serialised
+(`clonePlain`) while hashing the in-memory spec, so the item it returns
+disagreed with its own hash. `SysMetadataRepository` inherits the fix through
+the same function.
+
+Downstream, an incoherent version meant a repository could report an
+`{op:'update', actor:'fs'}` for a file nothing outside the process had touched:
+the head index held a hash the disk could never reproduce, so re-reading one's
+own write looked like somebody else's edit. That surfaces without any watcher —
+a restart rebuilds the index from disk and the version the caller was handed no
+longer matches it.
+
+**Ordinary specs hash exactly as before, and this is not a migration.** The new
+path diverges only at a position carrying a callable `toJSON`; a graph without
+one is byte-identical through `canonicalize`. Verified against this repository's
+entire checked-in JSON corpus — 1973 files hashed under both the old and the new
+implementation, **0 hashes changed** — and the `hashSpec({})` regression guard
+in `metadata-core` is unmoved. Stored versions for ordinary specs keep their
+meaning. Versions for `toJSON`-carrying specs do change, and those are exactly
+the versions that never identified their stored bytes in the first place.
+
+Also supported as a consequence: a class instance with a `toJSON` now hashes as
+whatever it serialises to, rather than as its private fields. One without a
+`toJSON` still hashes as its own enumerable keys — which is what
+`JSON.stringify` writes for it.
+
+The pin is table-driven and lives in the shared repository contract suite, so
+every `MetadataRepository` implementation is held to it: `Date` at a key, `Date`
+under an array index, a class whose `toJSON` yields a string, an object literal
+carrying its own `toJSON`, a nested case, and a plain-JSON control row that
+proves the fix did not simply change every hash.

packages/metadata-core/src/canonicalize.ts

@@ -16,24 +16,76 @@
  * they cannot survive a JSON round trip.
  * 5. **Idempotence.** `canonicalize(canonicalize(x))` === `canonicalize(x)`.
  * 6. **Pure.** No side-effects, no mutation of input.
+ * 7. **Serialized-form identity (#7856).** The hash describes the bytes a
+ * value serialises to, never the in-memory object graph that produced
+ * them. Formally, for every `x` this module accepts:
+ *
+ * canonicalize(x) === canonicalize(JSON.parse(JSON.stringify(x)))
+ *
+ * This is the guarantee that makes a repository's
+ * `put(spec).version === get().hash` hold: `put` hashes the value it
+ * was handed, `get` hashes what it parsed back off the disk, and
+ * guarantee 7 says those are one hash.
+ *
+ * It is delivered by honouring `toJSON` exactly as `JSON.stringify`
+ * does — consulted once per position, its result serialised as-is and
+ * never re-consulted. Before #7856 `normalise` ignored `toJSON` and
+ * walked own enumerable keys instead, so a `Date` canonicalised to a
+ * key-less `{}` while the bytes on disk held an ISO string, and the two
+ * hashed differently: the version handed to a caller did not identify
+ * the bytes stored, and re-reading one's own write looked like an
+ * external edit.
+ *
+ * Values carrying no `toJSON` anywhere in the graph — ordinary specs —
+ * are untouched by this: their canonical form is byte-identical to what
+ * it was before, so no already-stored version changes meaning.
  *
  * Non-goals (deliberately not supported):
  *
- * - Functions, symbols, class instances. These have no canonical JSON
- * form. Callers must serialise out-of-band (e.g. for formula fields,
- * use the CEL string, not the compiled function).
+ * - Functions and symbols. These have no canonical JSON form. Callers must
+ * serialise out-of-band (e.g. for formula fields, use the CEL string,
+ * not the compiled function).
+ * - Class instances *without* a `toJSON`. One that has a `toJSON` is
+ * supported by guarantee 7 and hashes as whatever it serialises to;
+ * one that does not still hashes as its own enumerable keys, which is
+ * exactly what `JSON.stringify` writes for it.
  * - BigInt. Rejected because there is no agreed-upon JSON representation.
  */
 
 import { createHash } from 'node:crypto';
 
 /** Stable JSON serialisation. See module-level doc for guarantees. */
 export function canonicalize(value: unknown): string {
- return JSON.stringify(normalise(value));
+ // `''` is the key `JSON.stringify` hands a root-position `toJSON`.
+ return JSON.stringify(normalise(value, ''));
+}
+
+/**
+ * Resolve one position's value the way `JSON.stringify` does: if it carries
+ * a callable `toJSON`, that is what gets serialised.
+ *
+ * Applied ONCE per position, per the `SerializeJSONProperty` algorithm — the
+ * returned value is serialised as-is and is never itself re-examined for a
+ * `toJSON`. `normalise` therefore calls this on entry and then dispatches on
+ * the result, recursing (and so re-applying it) only for child positions.
+ */
+function resolveToJson(value: unknown, key: string): unknown {
+ if (value === null || typeof value !== 'object') return value;
+ const toJson = (value as { toJSON?: unknown }).toJSON;
+ if (typeof toJson !== 'function') return value;
+ return (toJson as (this: unknown, key: string) => unknown).call(value, key);
 }
 
-/** Convert a value into a canonical, JSON-serialisable form. */
-function normalise(value: unknown): unknown {
+/**
+ * Convert a value into a canonical, JSON-serialisable form.
+ *
+ * @param value Raw value at this position.
+ * @param key The position's key — `''` at the root, the property name
+ * inside an object, the stringified index inside an array.
+ * Passed to `toJSON` because `JSON.stringify` passes it.
+ */
+function normalise(rawValue: unknown, key: string): unknown {
+ const value = resolveToJson(rawValue, key);
  if (value === null) return null;
  if (typeof value === 'undefined') return undefined; // caller-dropped
  if (typeof value === 'number') {
@@ -51,7 +103,11 @@ function normalise(value: unknown): unknown {
  if (typeof value === 'string' || typeof value === 'boolean') return value;
 
  if (Array.isArray(value)) {
- return value.map(normalise);
+ // NOT `value.map(normalise)` — `Array.prototype.map` passes the index as
+ // the second argument, which is this function's `key` parameter. The
+ // index is the right key, but it has to arrive as the string
+ // `JSON.stringify` would hand a `toJSON`, not as a number.
+ return value.map((element, index) => normalise(element, String(index)));
  }
 
  // Plain object: sort keys, drop undefineds.
@@ -60,7 +116,7 @@ function normalise(value: unknown): unknown {
  const out: Record<string, unknown> = {};
  const keys = Object.keys(obj).sort();
  for (const k of keys) {
- const v = normalise(obj[k]);
+ const v = normalise(obj[k], k);
  if (typeof v === 'undefined') continue;
  out[k] = v;
  }

packages/metadata-core/src/contract-suite.ts

@@ -38,6 +38,64 @@ const refOf = (overrides: Partial<MetaRef> = {}): MetaRef => ({
 
 const spec = (label: string) => ({ label, columns: ['a', 'b'] });
 
+/** A value whose `toJSON` collapses it to something other than its own keys. */
+class Money {
+ constructor(
+ private readonly cents: number,
+ private readonly currency: string,
+ ) {}
+
+ toJSON(): string {
+ return `${(this.cents / 100).toFixed(2)} ${this.currency}`;
+ }
+}
+
+/**
+ * Spec shapes for the #7856 identity pin, spanning the ways a value's
+ * serialised form can differ from its in-memory object graph.
+ *
+ * Deliberately a TABLE and not one hand-picked `Date`. The defect was never
+ * about `Date` — it was about `canonicalize` walking own enumerable keys
+ * while the disk received whatever `JSON.stringify` produced — and a
+ * single-case pin is exactly what lets the next value in this class through.
+ * `Date` is merely its most reachable instance; a `toJSON` returning a
+ * string, an object, or sitting under an array index are the same defect
+ * wearing different clothes, and each row below failed differently before
+ * the fix (see the PR's per-arm table).
+ *
+ * The control row matters as much as the rest: it is what proves a fix here
+ * did not simply change every hash.
+ */
+const SERIALISATION_SHAPES: ReadonlyArray<{
+ label: string;
+ spec: () => Record<string, unknown>;
+}> = [
+ {
+ label: 'control — plain JSON, no toJSON anywhere in the graph',
+ spec: () => ({ label: 'Home', columns: ['a', 'b'], nested: { n: 1, ok: true, nil: null } }),
+ },
+ {
+ label: 'Date at a top-level key',
+ spec: () => ({ label: 'Home', createdAt: new Date('2024-01-01T00:00:00.000Z') }),
+ },
+ {
+ label: 'Date under an array index',
+ spec: () => ({ label: 'Audit', stamps: [new Date('2020-06-01T12:00:00.000Z')] }),
+ },
+ {
+ label: 'class instance whose toJSON collapses it to a string',
+ spec: () => ({ label: 'Price', amount: new Money(1250, 'USD') }),
+ },
+ {
+ label: 'object literal carrying its own toJSON',
+ spec: () => ({ label: 'Range', span: { toJSON: () => ({ from: 1, to: 9 }) } }),
+ },
+ {
+ label: 'toJSON nested inside an array element',
+ spec: () => ({ rows: [{ at: new Date('2022-02-02T02:02:02.000Z') }] }),
+ },
+];
+
 /** Drain at most `n` events from an async iterable with a timeout. */
 async function take<T>(iter: AsyncIterable<T>, n: number, timeoutMs = 1000): Promise<T[]> {
  const out: T[] = [];
@@ -104,6 +162,64 @@ export function runRepositoryContractTests(
  expect(got!.hash).toBe(hashSpec(got!.body));
  });
 
+ // ── #7856 — the version identifies the STORED BYTES ───────────
+ //
+ // Table-driven on purpose; see SERIALISATION_SHAPES. Every row
+ // asserts BOTH faces of the same invariant, because the two
+ // implementations in this repo broke DIFFERENT ones:
+ //
+ // put().version === get().hash
+ // the face FileSystemRepository broke — it hashed the spec it
+ // was handed, wrote `JSON.stringify` of it, and re-hashed the
+ // parse on the way back out.
+ // get().hash === hashSpec(get().body)
+ // invariant 4's face, the one InMemoryRepository broke — it
+ // stores `body` already serialised (`clonePlain`) while
+ // hashing the in-memory spec, so the item it hands back
+ // disagrees with its own hash.
+ //
+ // Asserting only one face would have left the other implementation's
+ // divergence unpinned, which is the whole reason this lives in the
+ // shared contract suite rather than beside either bug.
+ describe('serialized-form identity (#7856)', () => {
+ for (const shape of SERIALISATION_SHAPES) {
+ it(`version identifies the stored bytes — ${shape.label}`, async () => {
+ const repo = await factory();
+ const ref = refOf();
+ const put = await repo.put(ref, shape.spec(), {
+ parentVersion: null,
+ actor: 't',
+ });
+
+ const got = await repo.get(ref);
+ expect(got).not.toBeNull();
+ expect(got!.hash).toBe(put.version);
+ expect(got!.hash).toBe(hashSpec(got!.body));
+ });
+
+ it(`re-putting the same spec is a no-op — ${shape.label}`, async () => {
+ const repo = await factory();
+ const ref = refOf();
+ const first = await repo.put(ref, shape.spec(), {
+ parentVersion: null,
+ actor: 't',
+ });
+
+ // Chains on the version the caller was HANDED. When that
+ // version does not identify the stored bytes, the head index
+ // it is compared against holds the other hash and this second
+ // write is not recognised as the no-op it is — it either
+ // conflicts or republishes the item as a fresh revision.
+ const second = await repo.put(ref, shape.spec(), {
+ parentVersion: first.version,
+ actor: 't',
+ });
+ expect(second.version).toBe(first.version);
+ expect(second.seq).toBe(first.seq);
+ });
+ }
+ });
+
  it('returns null for missing item', async () => {
  const repo = await factory();
  expect(await repo.get(refOf({ name: 'never_existed' }))).toBeNull();

packages/metadata-core/test/canonicalize.test.ts

@@ -147,4 +147,86 @@ describe('hashSpec', () => {
  { numRuns: 100 },
  );
  });
+
+ /**
+ * Guarantee 7 (#7856): the hash describes the bytes a value serialises to.
+ *
+ * canonicalize(x) === canonicalize(JSON.parse(JSON.stringify(x)))
+ *
+ * Stated as a property and a table rather than as a list of golden hashes,
+ * because the defect was never about any particular value — it was about
+ * `normalise` walking own enumerable keys while the disk received
+ * `JSON.stringify`'s output. Everything with a `toJSON` diverged; `Date` is
+ * simply the instance everybody meets first.
+ */
+ describe('serialized-form identity (#7856)', () => {
+ const roundTrip = (x: unknown): unknown => JSON.parse(JSON.stringify(x));
+
+ class Money {
+ constructor(
+ private readonly cents: number,
+ private readonly currency: string,
+ ) {}
+
+ toJSON(): string {
+ return `${(this.cents / 100).toFixed(2)} ${this.currency}`;
+ }
+ }
+
+ const CASES: ReadonlyArray<{ label: string; value: unknown; canonical: string }> = [
+ {
+ label: 'Date at a key',
+ value: { createdAt: new Date('2024-01-01T00:00:00.000Z'), label: 'H' },
+ canonical: '{"createdAt":"2024-01-01T00:00:00.000Z","label":"H"}',
+ },
+ {
+ label: 'Date under an array index',
+ value: { stamps: [new Date('2020-06-01T12:00:00.000Z')] },
+ canonical: '{"stamps":["2020-06-01T12:00:00.000Z"]}',
+ },
+ {
+ label: 'class instance whose toJSON yields a string',
+ value: { amount: new Money(1250, 'USD') },
+ canonical: '{"amount":"12.50 USD"}',
+ },
+ {
+ label: 'object literal carrying its own toJSON',
+ value: { span: { toJSON: () => ({ to: 9, from: 1 }) } },
+ canonical: '{"span":{"from":1,"to":9}}',
+ },
+ ];
+
+ for (const c of CASES) {
+ it(`canonicalises to the serialised form — ${c.label}`, () => {
+ expect(canonicalize(c.value)).toBe(c.canonical);
+ expect(hashSpec(c.value)).toBe(hashSpec(roundTrip(c.value)));
+ });
+ }
+
+ it('applies toJSON once per position, never re-consulting its result', () => {
+ // `JSON.stringify` applies toJSON exactly once per position, so a result
+ // that itself carries a toJSON is serialised literally. Matching
+ // JSON.stringify IS the contract, so it is asserted against it directly.
+ const value = { wrapped: { toJSON: () => ({ inner: { toJSON: () => 'deep' } }) } };
+ expect(canonicalize(value)).toBe(JSON.stringify(roundTrip(value)));
+ });
+
+ it('leaves a graph with no toJSON byte-identical', () => {
+ // The half that makes this a fix rather than a migration: an ordinary
+ // spec's canonical form must be exactly what it has always been.
+ expect(canonicalize({ b: 1, a: [1, 2, { c: null }], d: 'x' })).toBe(
+ '{"a":[1,2,{"c":null}],"b":1,"d":"x"}',
+ );
+ });
+
+ it('property: hashing is invariant under a JSON round trip', () => {
+ fc.assert(
+ fc.property(
+ fc.dictionary(fc.string({ minLength: 1, maxLength: 6 }), fc.jsonValue()),
+ (obj) => hashSpec(obj) === hashSpec(roundTrip(obj)),
+ ),
+ { numRuns: 200 },
+ );
+ });
+ });
 });