Repository navigation
fix(metadata): DatabaseLoader stamps and compares hashSpec(body, type), so a field-reorder-only register is persisted (#21828) - #21852
Conversation
…), so a field reorder is persisted The loader stamped sys_metadata with calculateChecksum (bare hex, every map sorted) and skipped its write when the new stamp equalled the stored one, so a register whose only change was the order of an object's fields was never persisted. It now stamps the hash SysMetadataRepository stamps on the same column, hashSpec(body, type), and decides "unchanged" by re-hashing the stored body under that rule, so a row stamped under an older rule is not rewritten when its body is unchanged. The history write takes the parent row's stamp and no longer compares stamps. Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 17 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin f76b47fb44989b6fe7f1cbe14197005dba070cee && git checkout f76b47fb44989b6fe7f1cbe14197005dba070cee
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 48297ad980b1137e8db1df46729f9890d0f0608e cf0537aa0807e1a8da620578f3f3deba602a97da && git checkout -B drift-repro 48297ad980b1137e8db1df46729f9890d0f0608e && git merge --no-ff cf0537aa0807e1a8da620578f3f3deba602a97da
node scripts/docs-audit/affected-docs.mjs --json 48297ad980b1137e8db1df46729f9890d0f0608e
|
ACCEPT (seat review) — PR #21852 at head
|
Fixes #21828
Clause-②: no
What was wrong
DatabaseLoader.savestampedsys_metadata.checksumwithcalculateChecksum(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 itsfieldshashed equal, soMetadataManager.registerrefreshed 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 theregisterpath.Reproduced at
8832655af2before the change (scratch probe, not committed): aDatabaseLoader.saveof{ fields: { amount, title } }over a row holding{ fields: { title, amount } }left version 1, one history row, and stored ordertitle,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.savecreate and update,registerRollback) stampscontentHash(json, type), which ishashSpec(JSON.parse(json), type)from@objectstack/metadata-core. That is the hashSysMetadataRepositorystamps 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, anundefinedproperty) is dropped as the stored column drops it, andhashSpecis never handed what it refuses.savedecides "unchanged" by re-hashing the STORED body underhashSpec(body, type)(storedBodyUnchanged), the comparisonSysMetadataRepository.putmakes for an ordered-map type. A stored stamp cannot answer it, because it may come from an older rule (H3).createHistoryRecordtakes 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 issave, 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).calculateChecksumkeeps its behaviour and its export (H4). Its docblock now says it is not thesys_metadata.checksumstamp.No
packages/spec/src/**edit, no export added, removed or changed. The builtdist/index.d.tsdiffers only in doc comments:DatabaseLoader.createHistoryRecordis private and emitted without a signature, andcontentHashandstoredBodyUnchangedare module-private. So no contract review is owed beyond what theClause-②: noline states.H1 — every checksum decision in the loader (at
8832655af2)save:1404,:1413–:1414calculateChecksum(data)vs the storedchecksumcontentHashstamp; content comparison viastoredBodyUnchangedsave:1433/:1466contentHashstampcreateHistoryRecord:725,:728calculateChecksum(metadata)vspreviousChecksum, for'update'onlycreateHistoryRecord:782,:799–:800checksum/previous_checksumto the history rowregisterRollback:1356,:1366,:1372calculateChecksum(restoredData); reads the stored stamp as previous; stamps the rowrevertrowcontentHashstamp; still no comparisonrowToRecord:874→load():1002,stat():1145etagloadreads noifNoneMatch)sha256:getHistoryRecord:1217,queryHistory:1322checksum/previous_checksumMetadataManager.diffpasses them through aschecksum1/checksum2and decidesidenticalfrom the patchH2 — two vocabularies in one column, measured
Scratch probe in
packages/metadata-protocol(deleted, never committed): one in-memory engine shared by aDatabaseLoaderand aSysMetadataRepository, each writer meeting the other's stamp on an unchanged body.8832655af29e2b5ea54d)saveover a repository-stampedviewsha256:2d77…to bare2d77…,updatehistory rowsaveover a repository-stampedobjectupdatehistory rowputover a loader-stampedviewsha256:,updatehistory rowsha256:2d77…putover a loader-stampedobjectsha256: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 firstregisterafter upgrade, with a version bump and anupdatehistory row. Ablation B measures exactly that. Sosavecompares content: a legacy-stamped row with an unchanged body is not rewritten and keeps its stamp until its content next changes. A reorder ofobject.fieldsis still a change, including a reorder into sorted key order against an order-blind stamp.H4, H5
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.MetadataManager.diff(pass-through), andSysMetadataRepository, which reads the stamp as its version token:rowToItem, the optimistic lock, and thestoredBodyUnchangedshort-circuit (H2). The REST metadata ETag is computed from the served body (protocol.tsgetMetaItemCached), not from the stored stamp.protocol.tsmigrate-storedhands the stored stamp back asstoredParentVersion, where it is compared with the same stored stamp. Outside the runtime, the offline probepackages/objectql/scripts/dry-run-hash-compat.tscompares the stored stamp with a type-blindhashSpec(body); see Acceptance notes.Pins
New file
packages/metadata/src/loaders/database-loader-21828-one-content-hash.test.ts: real SQLite (driver-sqlite-wasm), throughMetadataManager.register, read back from the rows.registerpersists the new order (version 2,create+updatehistory); the row and its history row carryhashSpec(body, 'object'), one value; a rollback restores the earlier order in the same vocabulary.object(keys around and insidefields) and aview.calculateChecksumwith an unchanged body is not rewritten (object and view); a reorder into sorted key order against an order-blindsha256:stamp is persisted.Reverse verification
Committed first (
9e2b5ea54d). Each mutation went throughnode scripts/ablation-replace.mjsin WRAP mode, with atrap … EXIT INT TERMrestore in the driver script. Each landing was proved on disk (anchor count 1 to 0, blob4da1f0736df3changed), and each restore was proved with blob equal to HEAD and an emptygit diff HEAD. The loader test imports./database-loader.jsfrom source, so no build was involved.saverestored to the pre-fix key-sortedcalculateChecksumstamp and stamp comparisonnewChecksum === previousChecksuminstead of the content comparisonControl 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-replacerefused (anchor count 1 to 1) and restored. It was re-anchored onconst historyId = generateId();and re-run. The table records the re-run.Tests
At
cf0537aa08(the final commit,origin/main2799155678merged; the merge touched onlypackages/spectest files):pnpm --filter @objectstack/metadata typecheck: exit 0. The new test is in the program:tsc --noEmit --listFilescounts it once.pnpm --filter @objectstack/metadata exec vitest run --maxWorkers=2: 58 files, 867 tests passed..d.tssignature, no spec), so per the dispatch contract only the package's own tests are owed. No other package's test exercisesDatabaseLoaderwrites or stamps: 10 files reference it, with zerosave/register/checksum/etaghits.Gates
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack(no paths) atcf0537aa08: 62 derived commands, the same 62 as at235e68165e. All 62 were run atcf0537aa08, every one exit 0, and--ranreconciles: "62 derived famil(ies) accounted for — 62 run, 0 NOT-MEASURED".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.cf0537aa08: 52 exit 0.check-closing-target-claim,check-partof-closing-keywordandcheck-single-claim-pathsanswered NOT WIRED (no pull-request context). They are re-run against this PR in the report on the card.check:adr-symbol-anchors,check:scripts-symbol-anchors,check:spec-docblock-symbol-anchors,check:adr-anchors, all exit 0.check:engine-double-contractasked for no pin row (no new double).pnpm exec eslint --no-inline-config --format jsonon the 3 changed.tsfiles at235e68165e(identical bytes atcf0537aa08) linted 3 files, with 0 errors and 0 warnings. Population: all three match the config'spackages/**/*.{ts,tsx,mts,cts}and**/*.{ts,tsx,mts,cts}objects. The changeset is in no lint glob. Invariance: every config object sets onlyecmaVersion/sourceType, with noparserOptions.projectand no typed rules, so this diff cannot move the verdict on any untouched file. The fullpnpm lintis CI's.Docs
No hand-written page states the loader's checksum rule.
content/docs/concepts/metadata-lifecycle.mdxstates thesha256:rule for the repository'sput(), and it is now also true of new loader writes. No doc edit.Acceptance notes
SysMetadataRepository.putanswers "unchanged" for a type with no ordered map by comparing stamps. So an unchanged-bodyputover a row the loader stamped before this release (bare hex) still rewrites it once, with a version bump and anupdatehistory row (H2, row 3, at8832655af2). 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 aDatabaseLoader(onlynew MetadataManager({ datasource, driver })orsetDataEnginedoes, and nothing in this repository calls either). Not filed.packages/objectql/scripts/dry-run-hash-compat.tscompares a stored stamp with a type-blindhashSpec(body). So it reportschecksum_driftfor everyobjectrow whosefieldsare not in sorted order, whichever writer stamped it since metadata: publishing a field reorder from the object designer is a silent no-op — the content hash is key-order-insensitive, so a reorderedfieldsmap hashes equal and the draft is dropped #21790. Offline probe, not filed.MetadataManager.save(type, …)passestypeunfolded, whereregisterfolds it withcanonicalMetadataServiceType. 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