Uh oh!
There was an error while loading. Please reload this page.
fix(metadata-core,metadata-fs): hash the serialized form, so put().version identifies the bytes actually stored (#7856) - #7992
Conversation
…sion identifies stored bytes (#7856) `canonicalize` walked a value's own enumerable keys, so a `Date` canonicalised to `{}` while `JSON.stringify` — what every repository actually writes — turned it into an ISO string. The hash of the in-memory spec and the hash of the bytes on disk were therefore different hashes for the same item. `normalise` now honours `toJSON` exactly as `JSON.stringify` does: consulted once per position, its result serialised as-is and never re-consulted. That makes canonicalize(x) === canonicalize(JSON.parse(JSON.stringify(x))) true by construction, which is what restores `put().version === get().hash`. Both repository implementations were wrong in different places, so the fix is one function rather than two patches: FileSystemRepository broke put().version === get().hash, InMemoryRepository broke the contract suite's invariant 4 (item.hash === hashSpec(item.body)) because it stores `body` already serialised while hashing the in-memory spec. Ordinary specs are unaffected: the new path diverges only at a position carrying a callable `toJSON`. Verified over the repository's whole checked-in JSON corpus (1973 files hashed both ways, 0 hashes changed). The pin is table-driven and lives in the shared repository contract suite, so every MetadataRepository implementation is held to it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C1j5QbRXy7iHasccZkx55y
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 1 package(s): 2 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also reference the affected code. These are read-only:
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#7856
domain:metadataPM seat on the implementing dev's behalf. The dev pushedclaude/issue-7856-hashspec-serialized-formbut had no GitHub write access in its container (three of four sibling dispatches hit the same thing this shift — branches pushed, no PRs). Body below is assembled from the dev's own changeset; the dev's full report will be appended as a comment when it hands the text over. Flag any gap to me, not to them.Implements the triage seat's ruling of 2026-08-12 (comment
5263973459): option 2 — hash the serialized form — plus the head-index fix and a table-driven pin. Option 3 (document the incoherence) was rejected there; option 1 (reject non-round-tripping specs at the door) was deferred to its own card, not ridden here.The defect
hashSpeccanonicalised aDateto{}, becausecanonicalizewalked a value's own enumerable keys and aDatehas none.JSON.stringify— what every repository actually writes — turns the sameDateinto 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 oneDate:The change
canonicalizenow honourstoJSONexactly asJSON.stringifydoes — consulted once per position, its result serialised as-is and never re-consulted — which makes a new guarantee true by construction:⭐ Both repository implementations were wrong, in different places
This is why the fix is one function rather than two patches:
FileSystemRepositorybrokeput().version === get().hash— it hashed the spec it was handed, wroteJSON.stringifyof it, then re-hashed the parse on the way back out.InMemoryRepositorybroke the repository contract's invariant 4 (item.hash === hashSpec(item.body)) — it storesbodyalready serialised (clonePlain) while hashing the in-memory spec, so the item it returned disagreed with its own hash.SysMetadataRepositoryinherits 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 — this is not a migration
The PM brief made this a hard criterion: if ordinary specs churn, it is a migration, not a fix. The dev measured it at corpus scale rather than asserting it.
The new path diverges only at a position carrying a callable
toJSON; a graph without one is byte-identical throughcanonicalize. Stored versions for ordinary specs keep their meaning. Versions fortoJSON-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
toJSONnow hashes as whatever it serialises to, rather than as its private fields. One without atoJSONstill hashes as its own enumerable keys — which is whatJSON.stringifywrites for it.The pin
Table-driven, as the card demanded (⛔ not a single hand-picked case), and placed in the shared repository contract suite so every
MetadataRepositoryimplementation is held to it — a stronger placement than the brief asked for. Rows:Dateat a key,Dateunder an array index, a class whosetoJSONyields a string, an object literal carrying its owntoJSON, a nested case, and a plain-JSON control row that proves the fix did not simply change every hash.Generated by Claude Code