fix(migration): build the v4→v5 canonical structure seed for decoded records

[kriszyp]

Summary

copyDb's structure observer seeds the canonical v5 shared-structures dictionary by encoding shapeForStructure(record) for each migrated record. The v4→v5 migration reads source records as RecordObject instances (the encoder's structPrototype), not plain Object, so the old gate value.constructor === Object stubbed every record to the scalar 1. The observer then minted no structure, canonicalStructures stayed undefined, the persist guard skipped, and the canonical seed was never written. v5 workers then mint the dictionary from an empty durable in their own encounter order and fork it — records decode against a divergent in-memory shape (Data read, but end of buffer not reached / silent wrong values) under v4→v5 upgrade load.

Fix: broaden shapeForStructure to recurse any non-leaf object (decoded records and plain objects) while still stubbing leaf object types (Blob, Date, Buffer/typed arrays, ArrayBuffer, Map, Set). A Blob must not be walked — that would pull the file-backed payload the skeleton exists to avoid. RecordObject's own-enumerable keys are exactly its data fields (methods are non-enumerable, metadata rides a WeakMap), so the seeded structure matches what msgpackr encodes for the real record.

Why this is the fix (#1508)

This is the prevention side of the v4→v5 structure-id fork family (#1453 / #1163; #1506 is the recovery side). Root cause confirmed live: a single-node 4.7.34 → 5.1.14 migration of 300 seeded nested records logged canonicalStructures=undefined, WILL_PERSIST=false, and shapeForStructure(record) → 1 for a real RecordObject. With the fix the observer mints the structures and the seed persists.

Where to look

• bin/copyDb.ts shapeForStructure — the broadened recursion gate. The risk to weigh is shape parity: the skeleton must encode to the same struct (key set + order) msgpackr produces for the real record, or it would itself fork the dictionary. It uses own-enumerable keys (matching struct fields) and stubs leaf object types so a binary/blob/date field stays a scalar leaf.

Tests

• unitTests/bin/shapeForStructure.test.js — guards the recursion (record vs plain vs leaf), own-enumerable-only iteration, and the leaf stubs. Fails on main (record → 1), passes with the fix.
• integrationTests/upgrade/4.x-upgrade.test.ts (real harperdb@4 source, runs in CI) passes with the fix — migration round-trips and records decode after cold restart (no regression). Verified locally; with the fix the migrated default CF contains the [Symbol.for('structures'), 'widgets/'] seed.
• End-to-end validated by a live 4.7.34 → 5.1.14 migration: with the fix the observer mints structures and the seed persists — canonicalStructures is a 19-entry array, WILL_PERSIST=true (vs undefined/skipped on main).
• Why no dedicated unit/CI seed-assertion: every unit-harness source path (warm cache, cold reopen, fixture-via-table()) decodes records to plain Object, not the cold RecordObjects a real v4 source yields; and at the integration layer copyStructures already writes a (verbatim v4) seed on main, so a simple "seed exists" check can't cleanly separate the fix from main. The multi-worker dictionary fork the seed prevents is validated by the cluster repro. The shapeForStructure unit test is the clean fail-without-fix guard.

Cross-model review

Codex + Gemini (agy) + a Harper-domain pass. No open blockers. Two findings were addressed in this diff: raw ArrayBuffer/SharedArrayBuffer would be walked (now excluded), and for...in prototype-chain leakage (now own-enumerable only). Adjudicated as not-blocking for migration data (acyclic msgpack values; Blob resolves to the global base; a literal __proto__ field encodes identically on both sides): cyclic-reference recursion (pre-existing; the old code recursed objects too) and the __proto__ own-key trap.

🤖 Generated by KrAIs (Claude Opus 4.8). LLM-authored; please review with that in mind.

[claude]

Reviewed; no blockers found.

[gemini-code-assist]

Code Review

This pull request updates the shapeForStructure function in bin/copyDb.ts to correctly recurse into decoded records (non-plain objects) during database migrations, while continuing to stub leaf object types like Blobs, Dates, and ArrayBuffers. It also introduces a comprehensive regression test suite in unitTests/bin/shapeForStructure.test.js. The review feedback recommends explicitly handling SharedArrayBuffer as a leaf object type (and adding it to the test suite) and optimizing the property iteration by using Object.keys() instead of a for...in loop with hasOwnProperty.

[heskew]

Map/Set probably should not be leaves here. msgpackr still records structures for object keys/values inside them; for example { m: new Map([["k", { a: 1, b: 2 }]]) } produces both ["m"] and ["a", "b"]. Since Any fields can carry these through migration, the canonical seed can still be incomplete and workers can mint the missing nested structures in different orders.

Suggested fix: recurse Map entries and Set values through shapeForStructure while preserving the container type, and add a regression case for object values inside Map/Set.

---

🤖 Posted by Codex (gpt-5.5) on Nathan's behalf

[ldt1996]

manual tests consistently showed this is the right fix, great work!