Skip to content

Commit 0fe0a59

Browse files
fix(metadata): DatabaseLoader stamps and compares hashSpec(body, type), so a field-reorder-only register is persisted (#21828) (#21852)
Fixes #21828 Clause-②: no ## What was wrong `DatabaseLoader.save` stamped `sys_metadata.checksum` with `calculateChecksum(data)`, which sorts every map before hashing, and skipped its write whenever that stamp equalled the stored one. An object whose only change was the order of its `fields` hashed equal, so `MetadataManager.register` refreshed the loader's cache with the new order while the persisted row kept the old one. This is the class of defect #21790 repaired on the designer's path, here on the `register` path. Reproduced at `8832655af2` before the change (scratch probe, not committed): a `DatabaseLoader.save` of `{ fields: { amount, title } }` over a row holding `{ fields: { title, amount } }` left version 1, one history row, and stored order `title,amount`. ## The fix Landing: `packages/metadata/src/loaders/database-loader.ts`, as the claim predicted. The producer of the stamp is the loader itself, so nothing moved to another package. - **One stamp.** Every loader write (`save` create and update, `registerRollback`) stamps `contentHash(json, type)`, which is `hashSpec(JSON.parse(json), type)` from `@objectstack/metadata-core`. That is the hash `SysMetadataRepository` stamps on the same column, so the column carries one vocabulary (`sha256:` + 64 hex). It hashes the JSON the loader stores, so a value JSON cannot carry (a function, an `undefined` property) is dropped as the stored column drops it, and `hashSpec` is never handed what it refuses. - **One comparison, about content.** `save` decides "unchanged" by re-hashing the STORED body under `hashSpec(body, type)` (`storedBodyUnchanged`), the comparison `SysMetadataRepository.put` makes for an ordered-map type. A stored stamp cannot answer it, because it may come from an older rule (H3). - **The history write no longer compares stamps.** `createHistoryRecord` takes the parent row's new stamp from its caller, so the history row carries the same value. Its own `'update'` comparison (new stamp vs previous stored stamp) is removed. Its only `'update'` caller is `save`, which reaches it after the content comparison. A second, stamp-based comparison can only be wrong: against an order-blind stamp, a reorder into sorted key order hashes equal and would get no history row (ablation C below). - ⛔ No second order-aware checksum, no order key: the map already holds the order, per triage. - `calculateChecksum` keeps its behaviour and its export (H4). Its docblock now says it is not the `sys_metadata.checksum` stamp. No `packages/spec/src/**` edit, no export added, removed or changed. The built `dist/index.d.ts` differs only in doc comments: `DatabaseLoader.createHistoryRecord` is private and emitted without a signature, and `contentHash` and `storedBodyUnchanged` are module-private. So no contract review is owed beyond what the `Clause-②: no` line states. ## H1 — every checksum decision in the loader (at `8832655af2`) | Site | Computes, compares or exposes | On equality | After this PR | |---|---|---|---| | `save` `:1404`, `:1413`–`:1414` | `calculateChecksum(data)` vs the stored `checksum` | No write; cache set to the new body; success | `contentHash` stamp; content comparison via `storedBodyUnchanged` | | `save` `:1433` / `:1466` | Stamps the row on update / create | n/a | `contentHash` stamp | | `createHistoryRecord` `:725`, `:728` | `calculateChecksum(metadata)` vs `previousChecksum`, for `'update'` only | No history row | Takes the caller's stamp; no comparison | | `createHistoryRecord` `:782`, `:799`–`:800` | Writes `checksum` / `previous_checksum` to the history row | n/a | Same columns, same value as the parent row | | `registerRollback` `:1356`, `:1366`, `:1372` | `calculateChecksum(restoredData)`; reads the stored stamp as previous; stamps the row | No comparison: always writes and appends one `revert` row | `contentHash` stamp; still no comparison | | `rowToRecord` `:874` → `load()` `:1002`, `stat()` `:1145` | Exposes the stored stamp as `etag` | Nothing compares it in the loader (`load` reads no `ifNoneMatch`) | Unchanged; a row written from now on reports `sha256:` | | `getHistoryRecord` `:1217`, `queryHistory` `:1322` | Expose history `checksum` / `previous_checksum` | `MetadataManager.diff` passes them through as `checksum1` / `checksum2` and decides `identical` from the patch | Unchanged | ## H2 — two vocabularies in one column, measured Scratch probe in `packages/metadata-protocol` (deleted, never committed): one in-memory engine shared by a `DatabaseLoader` and a `SysMetadataRepository`, each writer meeting the other's stamp on an unchanged body. | Case | At `8832655af2` | After this PR (`9e2b5ea54d`) | |---|---|---| | Loader `save` over a repository-stamped `view` | Rewritten: version 1 to 2, stamp `sha256:2d77…` to bare `2d77…`, `update` history row | No write, version 1, 1 history row | | Loader `save` over a repository-stamped `object` | Rewritten: version 1 to 2, `update` history row | No write | | Repository `put` over a loader-stamped `view` | Rewritten: version 1 to 2, bare to `sha256:`, `update` history row | No write: both writers stamp `sha256:2d77…` | | Repository `put` over a loader-stamped `object` | No write: the repository re-hashes the stored body for an ordered-map type | No write: both stamp `sha256:3c7d…` | So before this PR each writer's first unchanged write over the other's row was a phantom version bump with a history row recording no change. The view's digest was the same hex in both writers, `sha256:` prefix aside, because a type with no ordered map canonicalizes to the same sorted JSON. ## H3 — the upgrade without phantom bumps After the change a row the loader stamped before this release carries a bare-hex stamp that no `sha256:` hash equals. Comparing stamps would therefore rewrite every such row on the first `register` after upgrade, with a version bump and an `update` history row. Ablation B measures exactly that. So `save` compares content: a legacy-stamped row with an unchanged body is not rewritten and keeps its stamp until its content next changes. A reorder of `object.fields` is still a change, including a reorder into sorted key order against an order-blind stamp. ## H4, H5 - **H4.** `calculateChecksum` (`index.ts:45`) is unchanged and still exported. Nothing in this repository calls it any more except the new test, which uses it to produce a legacy stamp. - **H5.** Readers of a stored loader stamp: the two loader ETags and the two history readers in H1, `MetadataManager.diff` (pass-through), and `SysMetadataRepository`, which reads the stamp as its version token: `rowToItem`, the optimistic lock, and the `storedBodyUnchanged` short-circuit (H2). The REST metadata ETag is computed from the served body (`protocol.ts` `getMetaItemCached`), not from the stored stamp. `protocol.ts` `migrate-stored` hands the stored stamp back as `storedParentVersion`, where it is compared with the same stored stamp. Outside the runtime, the offline probe `packages/objectql/scripts/dry-run-hash-compat.ts` compares the stored stamp with a type-blind `hashSpec(body)`; see Acceptance notes. ## Pins New file `packages/metadata/src/loaders/database-loader-21828-one-content-hash.test.ts`: real SQLite (`driver-sqlite-wasm`), through `MetadataManager.register`, read back from the rows. - §A a field-reorder-only `register` persists the new order (version 2, `create` + `update` history); the row and its history row carry `hashSpec(body, 'object')`, one value; a rollback restores the earlier order in the same vocabulary. - §B an unordered map's key swap compares equal: no write and no history row, for an `object` (keys around and inside `fields`) and a `view`. - §C the upgrade: a row stamped by the old `calculateChecksum` with an unchanged body is not rewritten (object and view); a reorder into sorted key order against an order-blind `sha256:` stamp is persisted. ## Reverse verification Committed first (`9e2b5ea54d`). Each mutation went through `node scripts/ablation-replace.mjs` in WRAP mode, with a `trap … EXIT INT TERM` restore in the driver script. Each landing was proved on disk (anchor count 1 to 0, blob `4da1f0736df3` changed), and each restore was proved with blob equal to HEAD and an empty `git diff HEAD`. The loader test imports `./database-loader.js` from source, so no build was involved. | Ablation | Mutation | Red | Green | |---|---|---|---| | A | `save` restored to the pre-fix key-sorted `calculateChecksum` stamp and stamp comparison | §A reorder persists, §A one-vocabulary stamp, §C reorder against an order-blind stamp (3) | §A rollback, both §B key-swap pins, §C legacy unchanged ×2 (5) | | B | New stamp, but `newChecksum === previousChecksum` instead of the content comparison | all three §C upgrade pins (3) | §A ×3, §B ×2 (5) | | C | The history write's stamp comparison restored | §C reorder against an order-blind stamp (history 1, not 2) | the other 7 | Control on the unmutated HEAD, run after each driver pass: 8/8 green. The first attempt at C was a no-op: its replacement still contained its anchor, so `ablation-replace` refused (anchor count 1 to 1) and restored. It was re-anchored on `const historyId = generateId();` and re-run. The table records the re-run. ## Tests At `cf0537aa08` (the final commit, `origin/main` `2799155678` merged; the merge touched only `packages/spec` test files): - `pnpm --filter @objectstack/metadata typecheck`: exit 0. The new test is in the program: `tsc --noEmit --listFiles` counts it once. - `pnpm --filter @objectstack/metadata exec vitest run --maxWorkers=2`: 58 files, 867 tests passed. - Consumers: no public surface moved (no export, no `.d.ts` signature, no spec), so per the dispatch contract only the package's own tests are owed. No other package's test exercises `DatabaseLoader` writes or stamps: 10 files reference it, with zero `save`/`register`/`checksum`/`etag` hits. ## Gates `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` (no paths) at `cf0537aa08`: 62 derived commands, the same 62 as at `235e68165e`. All 62 were run at `cf0537aa08`, every one exit 0, and `--ran` reconciles: "62 derived famil(ies) accounted for — 62 run, 0 NOT-MEASURED". - Beyond the dispatch list, the change set added: `check-adr-0087-registration` ×2, `check-empty-changeset` ×2, `release-rehearsal-clone --self-test`, `release-pending-publish --self-test`, `check:engine-double-contract`, `check:objectql-double-limit`, `check:objectui-changeset`, `check:pm-changeset-deadline-census`, `check:query-options-erasure`, `check:type-check-coverage`, `check:type-check-debt`, `check:where-matcher`. All exit 0. - Artifact-roster block (55 printed outside the total), at `cf0537aa08`: 52 exit 0. `check-closing-target-claim`, `check-partof-closing-keyword` and `check-single-claim-paths` answered NOT WIRED (no pull-request context). They are re-run against this PR in the report on the card. - The four symbol-anchor sweeps: `check:adr-symbol-anchors`, `check:scripts-symbol-anchors`, `check:spec-docblock-symbol-anchors`, `check:adr-anchors`, all exit 0. - `check:engine-double-contract` asked for no pin row (no new double). - Lint, narrowed and declared. `pnpm exec eslint --no-inline-config --format json` on the 3 changed `.ts` files at `235e68165e` (identical bytes at `cf0537aa08`) linted 3 files, with 0 errors and 0 warnings. Population: all three match the config's `packages/**/*.{ts,tsx,mts,cts}` and `**/*.{ts,tsx,mts,cts}` objects. The changeset is in no lint glob. Invariance: every config object sets only `ecmaVersion` / `sourceType`, with no `parserOptions.project` and no typed rules, so this diff cannot move the verdict on any untouched file. The full `pnpm lint` is CI's. ## Docs No hand-written page states the loader's checksum rule. `content/docs/concepts/metadata-lifecycle.mdx` states the `sha256:` rule for the repository's `put()`, and it is now also true of new loader writes. No doc edit. ## Acceptance notes - **Residual on the repository side, for rows stamped before this release.** `SysMetadataRepository.put` answers "unchanged" for a type with no ordered map by comparing stamps. So an unchanged-body `put` over a row the loader stamped before this release (bare hex) still rewrites it once, with a version bump and an `update` history row (H2, row 3, at `8832655af2`). New loader stamps match, so this is confined to pre-release rows and converges after one write each. Reach not measured: no in-repo host composes a `DatabaseLoader` (only `new MetadataManager({ datasource, driver })` or `setDataEngine` does, and nothing in this repository calls either). Not filed. - `packages/objectql/scripts/dry-run-hash-compat.ts` compares a stored stamp with a type-blind `hashSpec(body)`. So it reports `checksum_drift` for every `object` row whose `fields` are not in sorted order, whichever writer stamped it since #21790. Offline probe, not filed. - `MetadataManager.save(type, …)` passes `type` unfolded, where `register` folds it with `canonicalMetadataServiceType`. A plural spelling would hash with no ordered-map row and key a separate row. Read from source, unexercised, not filed. --- _Generated by [Claude Code](https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 18fe681 commit 0fe0a59

4 files changed

Lines changed: 299 additions & 14 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/metadata': patch
3+
---
4+
5+
`DatabaseLoader` now persists a `register` whose only change is the order of an object's `fields` (#21828). Before, the loader's checksum sorted every map, so a field reorder hashed equal to the stored row and was never written. The running process still saw the new order, but the persisted `sys_metadata` row kept the old one.
6+
7+
Clause-②: no
8+
9+
- **One hash vocabulary in `sys_metadata.checksum`.** The loader now stamps the hash `SysMetadataRepository` stamps on the same column: `hashSpec(body, type)` from `@objectstack/metadata-core`, written as `sha256:` + 64 hex. It is hashed as the item's metadata type, so a reorder of an object's `fields` is a change and every other map is still key-order independent. Its history rows carry the same value as the row they record. Before, the loader wrote bare hex from `calculateChecksum`. That function is unchanged and still exported, but nothing writes the column with it.
10+
- **Rows stamped before this release.** Whether a `register` is a no-op is now decided by re-hashing the stored body, not by comparing the stored checksum. A row with an unchanged body is not rewritten: no version bump, no history row, and it keeps its old checksum until its content next changes. The first real change rewrites it with the new stamp. A reorder into sorted key order is written too, even against a stored checksum that sorted every map.
11+
- **ETags.** `load()` and `stat()` report the row's checksum as `etag`, so a row the loader writes from this release on reports a `sha256:` value.
Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#21828] `DatabaseLoader` stamps and compares the content hash
5+
* `SysMetadataRepository` stamps on the same `sys_metadata` column,
6+
* `hashSpec(body, type)`, so the column carries one vocabulary and a
7+
* field-reorder-only `register` is persisted.
8+
*
9+
* Before this card the loader stamped `calculateChecksum` (bare hex, every map
10+
* sorted) and skipped its write whenever the new stamp equalled the stored
11+
* one. An `object` whose only change was the order of its `fields` hashed
12+
* equal, so `MetadataManager.register` refreshed the in-process cache with
13+
* the new order and the persisted row kept the old one.
14+
*
15+
* Driven through the door the card names, `MetadataManager.register`, onto
16+
* real SQLite (`driver-sqlite-wasm`, already a devDependency), and read back
17+
* from the rows, never from the loader's cache.
18+
*
19+
* Pins, per triage's grade on the card:
20+
* §A a field-reorder-only `register` persists the new order;
21+
* §B an unordered map's key swap still compares equal (no write, no history row);
22+
* §C the upgrade: a row stamped under an older rule with an unchanged body
23+
* is not rewritten, and a reorder against such a stamp is still persisted.
24+
*/
25+
26+
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
27+
import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
28+
import { hashSpec } from '@objectstack/metadata-core';
29+
import { DatabaseLoader } from './database-loader.js';
30+
import { MetadataManager } from '../metadata-manager.js';
31+
import { calculateChecksum } from '../utils/metadata-history-utils.js';
32+
33+
type Row = Record<string, unknown>;
34+
35+
const TITLE = { type: 'text', label: 'Title' };
36+
const AMOUNT = { type: 'number', label: 'Amount' };
37+
const DUE = { type: 'date', label: 'Due' };
38+
39+
/** An object whose `fields` are in the order given, and nothing else differs. */
40+
function invoice(order: ReadonlyArray<'title' | 'amount' | 'due'>): Row {
41+
const all = { title: TITLE, amount: AMOUNT, due: DUE };
42+
const fields: Row = {};
43+
for (const key of order) fields[key] = { ...all[key] };
44+
return { name: 'invoice', label: 'Invoice', fields };
45+
}
46+
47+
describe('[#21828] DatabaseLoader stamps and compares hashSpec(body, type)', () => {
48+
let driver: SqliteWasmDriver;
49+
let manager: MetadataManager;
50+
51+
beforeEach(async () => {
52+
driver = new SqliteWasmDriver({ filename: ':memory:' });
53+
await driver.connect();
54+
const loader = new DatabaseLoader({ driver, trackHistory: true, cache: { enabled: false } });
55+
manager = new MetadataManager({ datasource: 'sqlite', loaders: [loader] });
56+
});
57+
58+
afterEach(async () => {
59+
await driver.disconnect();
60+
});
61+
62+
async function row(type: string, name: string): Promise<Row> {
63+
const found = await driver.findOne('sys_metadata', { where: { type, name } });
64+
expect(found, `${type}/${name} has a sys_metadata row`).toBeTruthy();
65+
return found as Row;
66+
}
67+
68+
async function history(type: string, name: string): Promise<Row[]> {
69+
return (await driver.find('sys_metadata_history', { where: { type, name } })) as Row[];
70+
}
71+
72+
function storedFieldOrder(r: Row): string[] {
73+
return Object.keys((JSON.parse(r.metadata as string) as { fields: Row }).fields);
74+
}
75+
76+
/** Overwrite the stored stamp, as a row written under an older rule carries it. */
77+
async function restamp(r: Row, checksum: string): Promise<void> {
78+
await driver.update('sys_metadata', r.id as string, { checksum });
79+
}
80+
81+
describe('§A a field-reorder-only register is persisted', () => {
82+
it('stores the new order, bumps the version and appends one history row', async () => {
83+
await manager.register('object', 'invoice', invoice(['title', 'amount', 'due']));
84+
await manager.register('object', 'invoice', invoice(['due', 'title', 'amount']));
85+
86+
const r = await row('object', 'invoice');
87+
expect(storedFieldOrder(r)).toEqual(['due', 'title', 'amount']);
88+
expect(r.version).toBe(2);
89+
const h = await history('object', 'invoice');
90+
expect(h.map((x) => x.operation_type).sort()).toEqual(['create', 'update']);
91+
});
92+
93+
it('stamps the row and its history row with the repository vocabulary, one value', async () => {
94+
await manager.register('object', 'invoice', invoice(['title', 'amount', 'due']));
95+
const first = await row('object', 'invoice');
96+
await manager.register('object', 'invoice', invoice(['due', 'title', 'amount']));
97+
const r = await row('object', 'invoice');
98+
99+
expect(r.checksum).toBe(hashSpec(invoice(['due', 'title', 'amount']), 'object'));
100+
expect(r.checksum).not.toBe(first.checksum);
101+
const update = (await history('object', 'invoice')).find((x) => x.operation_type === 'update')!;
102+
expect(update.checksum).toBe(r.checksum);
103+
expect(update.previous_checksum).toBe(first.checksum);
104+
});
105+
106+
it('a rollback restores the earlier order and stamps it in the same vocabulary', async () => {
107+
await manager.register('object', 'invoice', invoice(['title', 'amount', 'due']));
108+
await manager.register('object', 'invoice', invoice(['due', 'title', 'amount']));
109+
await manager.rollback('object', 'invoice', 1);
110+
111+
const r = await row('object', 'invoice');
112+
expect(storedFieldOrder(r)).toEqual(['title', 'amount', 'due']);
113+
expect(r.checksum).toBe(hashSpec(invoice(['title', 'amount', 'due']), 'object'));
114+
});
115+
});
116+
117+
describe('§B a key swap in an unordered map still compares equal', () => {
118+
it('object: swapping the keys around `fields` and inside a field writes nothing', async () => {
119+
await manager.register('object', 'invoice', {
120+
name: 'invoice',
121+
label: 'Invoice',
122+
fields: { title: { type: 'text', label: 'Title' } },
123+
});
124+
const before = await row('object', 'invoice');
125+
await manager.register('object', 'invoice', {
126+
fields: { title: { label: 'Title', type: 'text' } },
127+
label: 'Invoice',
128+
name: 'invoice',
129+
});
130+
131+
const after = await row('object', 'invoice');
132+
expect(after.version).toBe(1);
133+
expect(after.checksum).toBe(before.checksum);
134+
expect(await history('object', 'invoice')).toHaveLength(1);
135+
});
136+
137+
it('view: a type with no ordered map stays order-blind in every map', async () => {
138+
await manager.register('view', 'invoice_grid', {
139+
name: 'invoice_grid',
140+
label: 'Invoices',
141+
options: { density: 'compact', striped: true },
142+
});
143+
await manager.register('view', 'invoice_grid', {
144+
options: { striped: true, density: 'compact' },
145+
label: 'Invoices',
146+
name: 'invoice_grid',
147+
});
148+
149+
const r = await row('view', 'invoice_grid');
150+
expect(r.version).toBe(1);
151+
expect(await history('view', 'invoice_grid')).toHaveLength(1);
152+
});
153+
});
154+
155+
describe('§C the upgrade: content, not a stamp from an older rule, decides "unchanged"', () => {
156+
it('a row stamped by the old calculateChecksum with an unchanged body is not rewritten', async () => {
157+
const body = invoice(['title', 'amount', 'due']);
158+
await manager.register('object', 'invoice', body);
159+
const legacy = await calculateChecksum(body);
160+
await restamp(await row('object', 'invoice'), legacy);
161+
162+
await manager.register('object', 'invoice', invoice(['title', 'amount', 'due']));
163+
164+
const r = await row('object', 'invoice');
165+
expect(r.version).toBe(1);
166+
expect(r.checksum).toBe(legacy);
167+
expect(await history('object', 'invoice')).toHaveLength(1);
168+
});
169+
170+
it('the same holds for a type with no ordered map', async () => {
171+
const body = { name: 'invoice_grid', label: 'Invoices' };
172+
await manager.register('view', 'invoice_grid', body);
173+
const legacy = await calculateChecksum(body);
174+
await restamp(await row('view', 'invoice_grid'), legacy);
175+
176+
await manager.register('view', 'invoice_grid', { ...body });
177+
178+
const r = await row('view', 'invoice_grid');
179+
expect(r.version).toBe(1);
180+
expect(r.checksum).toBe(legacy);
181+
expect(await history('view', 'invoice_grid')).toHaveLength(1);
182+
});
183+
184+
it('a reorder INTO sorted key order against an order-blind stamp is persisted', async () => {
185+
// `hashSpec(body)` with no type is the order-blind `sha256:` stamp a row
186+
// carries when `SysMetadataRepository` wrote it before #21790. It IS the
187+
// hash of the sorted order, so a stamp comparison calls this reorder
188+
// "unchanged".
189+
await manager.register('object', 'invoice', invoice(['title', 'due', 'amount']));
190+
await restamp(await row('object', 'invoice'), hashSpec(invoice(['title', 'due', 'amount'])));
191+
192+
await manager.register('object', 'invoice', invoice(['amount', 'due', 'title']));
193+
194+
const r = await row('object', 'invoice');
195+
expect(storedFieldOrder(r)).toEqual(['amount', 'due', 'title']);
196+
expect(r.version).toBe(2);
197+
expect(r.checksum).toBe(hashSpec(invoice(['amount', 'due', 'title']), 'object'));
198+
expect(await history('object', 'invoice')).toHaveLength(2);
199+
});
200+
});
201+
});

‎packages/metadata/src/loaders/database-loader.ts‎

Lines changed: 81 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,12 @@ import type {
1919
MetadataRecord,
2020
MetadataHistoryRecord,
2121
} from '@objectstack/spec/system';
22-
import { SysMetadataObject, SysMetadataHistoryObject } from '@objectstack/metadata-core';
22+
import { SysMetadataObject, SysMetadataHistoryObject, hashSpec } from '@objectstack/metadata-core';
2323
import { applyConversionsToStoredItem } from '@objectstack/spec';
2424
import { lowerFilterCondition, type FilterLoweringOptions } from '@objectstack/spec/data';
2525
import { PLURAL_TO_SINGULAR } from '@objectstack/spec/shared';
2626
import type { IDataDriver, IDataEngine, DriverQuery } from '@objectstack/spec/contracts';
2727
import type { MetadataLoader, MetadataKeyedItem } from './loader-interface.js';
28-
import { calculateChecksum } from '../utils/metadata-history-utils.js';
2928
import { LRUCache } from '../utils/lru-cache.js';
3029
// [commit 6a180e42d] Both predicates moved to `@objectstack/types` — see its
3130
// `driver-error-classification.ts` `## Home` section. The verdicts are
@@ -698,12 +697,23 @@ export class DatabaseLoader implements MetadataLoader {
698697
/**
699698
* Create a history record for a metadata change.
700699
*
700+
* [#21828] The caller has already decided this IS a change and hands in the
701+
* stamp it wrote on the parent row ({@link contentHash}), so the history row
702+
* carries the same value. ⛔ No "unchanged?" comparison here: the only
703+
* `'update'` caller, `save`, reaches this line after comparing CONTENT, and a
704+
* second comparison of the new stamp with the previous STORED stamp is never
705+
* the right arbiter. A stamp written under an older rule can equal the new
706+
* one while the content differs (a reorder of an object's `fields` into
707+
* sorted key order, against a stamp that sorted every map), which would
708+
* record a real change with no history row.
709+
*
701710
* @param type - Metadata type
702711
* @param name - Metadata name
703712
* @param version - Version number
704713
* @param metadata - The metadata payload
705714
* @param operationType - Type of operation
706-
* @param previousChecksum - Checksum of previous version (if any)
715+
* @param checksum - The parent row's new stamp, `contentHash` of the stored body
716+
* @param previousChecksum - The parent row's stamp before this write (if any)
707717
* @param changeNote - Optional change description
708718
* @param recordedBy - Optional user who made the change
709719
*/
@@ -713,6 +723,7 @@ export class DatabaseLoader implements MetadataLoader {
713723
version: number,
714724
metadata: unknown,
715725
operationType: 'create' | 'update' | 'publish' | 'revert' | 'delete',
726+
checksum: string,
716727
previousChecksum?: string,
717728
changeNote?: string,
718729
recordedBy?: string
@@ -722,12 +733,6 @@ export class DatabaseLoader implements MetadataLoader {
722733
await this.ensureHistorySchema();
723734

724735
const now = new Date().toISOString();
725-
const checksum = await calculateChecksum(metadata);
726-
727-
// Skip if checksum matches previous version (no actual change)
728-
if (previousChecksum && checksum === previousChecksum && operationType === 'update') {
729-
return;
730-
}
731736

732737
const historyId = generateId();
733738
const metadataJson = JSON.stringify(metadata);
@@ -1353,7 +1358,8 @@ export class DatabaseLoader implements MetadataLoader {
13531358

13541359
const now = new Date().toISOString();
13551360
const metadataJson = JSON.stringify(restoredData);
1356-
const newChecksum = await calculateChecksum(restoredData);
1361+
// [#21828] The one stamp vocabulary of `sys_metadata` — see {@link contentHash}.
1362+
const newChecksum = contentHash(metadataJson, type);
13571363

13581364
const existing = await this._findOne(this.tableName, {
13591365
where: this.baseFilter(type, name),
@@ -1383,6 +1389,7 @@ export class DatabaseLoader implements MetadataLoader {
13831389
newVersion,
13841390
restoredData,
13851391
'revert',
1392+
newChecksum,
13861393
previousChecksum,
13871394
changeNote ?? `Rolled back to version ${targetVersion}`,
13881395
recordedBy
@@ -1401,17 +1408,19 @@ export class DatabaseLoader implements MetadataLoader {
14011408

14021409
const now = new Date().toISOString();
14031410
const metadataJson = JSON.stringify(data);
1404-
const newChecksum = await calculateChecksum(data);
14051411

14061412
try {
1413+
// [#21828] The one stamp vocabulary of `sys_metadata` — see {@link contentHash}.
1414+
const newChecksum = contentHash(metadataJson, type);
14071415
const existing = await this._findOne(this.tableName, {
14081416
where: this.baseFilter(type, name),
14091417
});
14101418

14111419
if (existing) {
1412-
// Skip update if the content is identical (prevents phantom version bumps)
1420+
// Skip the write when the stored CONTENT is identical (prevents phantom
1421+
// version bumps). [#21828] Content, not stamps: see `storedBodyUnchanged`.
14131422
const previousChecksum = existing.checksum as string | undefined;
1414-
if (newChecksum === previousChecksum) {
1423+
if (storedBodyUnchanged(existing, type, newChecksum)) {
14151424
// No DB write, but make sure the cached payload reflects the latest
14161425
// call (prior cached `null` would otherwise mask a freshly-saved
14171426
// record).
@@ -1444,6 +1453,7 @@ export class DatabaseLoader implements MetadataLoader {
14441453
version,
14451454
data,
14461455
'update',
1456+
newChecksum,
14471457
previousChecksum
14481458
);
14491459

@@ -1481,7 +1491,8 @@ export class DatabaseLoader implements MetadataLoader {
14811491
name,
14821492
1,
14831493
data,
1484-
'create'
1494+
'create',
1495+
newChecksum
14851496
);
14861497

14871498
return {
@@ -1523,6 +1534,62 @@ export class DatabaseLoader implements MetadataLoader {
15231534
}
15241535
}
15251536

1537+
/**
1538+
* [#21828] The content hash this loader stamps on a `sys_metadata` row and on
1539+
* its history row: `hashSpec(body, type)` from `@objectstack/metadata-core`,
1540+
* the hash `SysMetadataRepository` stamps on the same column. One column, one
1541+
* vocabulary (`sha256:` + 64 hex). ⛔ Not `calculateChecksum`: it sorts every
1542+
* map, an object's `fields` included, and writes bare hex, so a field-reorder-
1543+
* only `register` hashed equal and was never persisted.
1544+
*
1545+
* Hashed AS ITS TYPE (canonicalize guarantee 8): a map the spec declares
1546+
* ordered (an `object`'s `fields`) keeps its key order, so a reorder of it is a
1547+
* change, and every other map stays key-order independent.
1548+
*
1549+
* It hashes the bytes the loader stores (the parse of the JSON it writes), not
1550+
* the in-memory value it was handed. Canonicalize guarantee 7 makes the two one
1551+
* hash for every value JSON can carry. For a value it cannot (a function or an
1552+
* `undefined` property, which the stored JSON drops), the stamp still names
1553+
* the stored bytes, and `hashSpec` is never handed what it refuses.
1554+
*/
1555+
function contentHash(storedJson: string, type: string): string {
1556+
return hashSpec(JSON.parse(storedJson), type);
1557+
}
1558+
1559+
/**
1560+
* [#21828] Does `row` already store the body whose {@link contentHash} is
1561+
* `hash`? `save`'s no-op question, answered about CONTENT: the stored bytes
1562+
* are re-hashed under the current rule, the comparison
1563+
* `SysMetadataRepository.put` makes for an ordered-map type
1564+
* (`storedBodyUnchanged` there).
1565+
*
1566+
* The stored stamp cannot answer it, because a row's stamp may come from an
1567+
* older rule. Before #21828 this loader stamped `calculateChecksum` (bare hex,
1568+
* every map sorted), and a row `SysMetadataRepository` stamped before #21790
1569+
* carries an order-blind `sha256:` hash. Trusting the stamp misfires both ways:
1570+
* - an identical body looks changed against a stamp no current hash equals,
1571+
* so every such row would be rewritten with a version bump and a history
1572+
* row recording no change, on the first `register` after an upgrade;
1573+
* - a reorder of an object's `fields` INTO sorted key order looks unchanged
1574+
* against an order-blind stamp, the drop #21828 fixes.
1575+
* The stamp of a skipped row is left as written: it stays the row's version
1576+
* token until its content next changes.
1577+
*
1578+
* Stored bytes that are absent or do not parse are not the incoming body, so
1579+
* the answer is "changed" and the write goes ahead, which also repairs the row.
1580+
*/
1581+
function storedBodyUnchanged(row: Record<string, unknown>, type: string, hash: string): boolean {
1582+
const stored = row.metadata;
1583+
if (stored === null || stored === undefined) return false;
1584+
let storedBody: unknown;
1585+
try {
1586+
storedBody = typeof stored === 'string' ? JSON.parse(stored) : stored;
1587+
} catch {
1588+
return false;
1589+
}
1590+
return hashSpec(storedBody, type) === hash;
1591+
}
1592+
15261593
/**
15271594
* Generate a simple unique ID for metadata records.
15281595
* Uses crypto.randomUUID when available, falls back to timestamp-based ID.

0 commit comments

Comments
 (0)