Skip to content

A spec that does not round-trip through JSON silently corrupts the metadata-fs head index — put().version already disagrees with get().hash, no watcher involved #7856

Description

@huangyiirene

Filed by the domain:metadata PM seat from #7335's dev report (PR #7845), which measured this while proving that removing the time-keyed self-write suppression could not regress it. Deliberately kept out of that PR: fixing it there would have masked a coherence bug behind a suppression change and made that PR's reverse verification attributable to two things at once.

No domain:* applied — routing is the triage seat's territory. For the record the surfaces are packages/metadata-core (hashSpec) and packages/metadata-fs (put() / the head index).

Measured, not inferred

hashSpec canonicalises a Date to {} in memory, but JSON.stringify writes it as an ISO string, so the hash of the in-memory spec and the hash of the bytes actually on disk differ. Measured on origin/main @ 69fde55:

in-memory hash : 2978612c
on-disk hash   : e5abe659

Two consequences, both present today with the watcher disabled entirely — so this is not a watcher bug and does not depend on any timing:

  1. put(spec).version !== get().hash for any spec carrying a Date (or any object with a toJSON that changes its serialised shape). The version a caller is handed does not identify the bytes that were stored.
  2. A second put() of such a spec publishes a spurious {op: 'update', actor: 'fs'} and corrupts the head index — the stored head never matches what the watcher subsequently reads, so the repository believes an external actor edited a file that nothing outside the process touched.

Why it surfaced now, and why it is not #7335

#7335's dispatch fenced "specs whose in-memory form does not round-trip through JSON identically" as the most likely way a content-identity comparison silently regresses. The dev checked, and the honest answer was that the hazard is entirely pre-existing and was never covered by the mechanism being removed: on the pre-fix tree the event arrives ~560 ms after the write, roughly 360 ms after the 200 ms suppression window had already expired. The old time-keyed pre-check therefore never held this class, and deleting it cannot make it worse.

That reasoning is what makes this a separate card rather than a regression of #7845 — and it is why the defect is worth its own measurement rather than being folded into a passing PR.

The actual question, which is a design one

There are at least three shapes and this card does not pick one:

  1. Reject at the doorput() refuses a spec that does not round-trip, loudly, naming the offending key. Consistent with "declared = enforced"; hardest for an AI author to get silently wrong; most likely to break existing callers.
  2. Normalise before hashing — hash the serialised form rather than the in-memory one, so version always identifies the stored bytes. Smallest blast radius; leaves authors free to pass values whose in-memory and stored forms differ, which may be its own trap.
  3. Accept and document — declare that version identifies the in-memory spec and not the bytes, and fix the head-index write instead.

Option 2 looks cheapest and option 1 fits the repo's contract-first posture; whoever takes it should measure how many real specs carry non-round-tripping values before assuming option 1's blast radius is tolerable. If the choice turns on product intent unreadable from the code, that is a needs-user-decision, not a coin flip.

⚠️ Whatever shape wins, the pin belongs where a future divergence is caught: assert put().version === get().hash over a table of spec shapes including at least one Date and one custom toJSON, not a single hand-picked case.

Provenance

#7335 · PR #7845 (1277a580) — see its "Card clauses / 2. Specs that do not round-trip through JSON" section, and handleFsChange, where the dev documented the behaviour rather than silently changing it.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions