Make cluster-origin table definitions additive-only so a peer's partial schema snapshot cannot destroy locally declared attributes

[kriszyp]

Make cluster-origin table definitions additive-only so a peer's partial schema snapshot cannot destroy locally declared attributes

What / why

table() treated every incoming attribute list as authoritative: the existing-table branch spliced Table.attributes down to the incoming list, and the catalog reconcile loop deleted the descriptors (and indexes) of any attribute the list omitted. That is correct for local schema authoring (create_table, @table, defineTable) — but replication also calls table() with definitions that are only a snapshot of a peer's eventually-consistent view: the DB_SCHEMA handshake announcement (harper-pro ensureTableIfChanged) and the replicated define_schema event (Table.ts). Such a snapshot can be captured mid-create on the peer (only the primary key registered yet) or applied by a worker whose thread-local databases map has not yet absorbed a concurrent local create_table.

When those two races lined up, a partial peer snapshot permanently destroyed attributes a local create_table had just successfully declared: the in-memory list was spliced down and the on-disk catalog descriptor deleted. Because the table was schemaDefined, the handshake's honor-local guard then refused to ever re-add the attribute from peers, so every later search_by_value failed with unknown attribute — the root cause of the harper-pro nightly replicationLoad failure (replicate across many databases, run 32457825567). The failing node's logs show create_table db3.test [id, name] on main/0 at 07:20:19.146 returning success, followed at 07:20:19.199 by http/1 (Re)creating { table:'test', schemaDefined:true, attributes:[{id}] } from a peer announcement, and from then on Schema for 'db3.test' is defined locally, but attribute 'name: String' from '127.0.0.5' does not match local attribute which does not exist until the search 500s.

The invariant this PR enforces: a definition with origin: 'cluster' is applied additively — peer attributes the local table lacks are added; locally declared attributes are never removed or redefined; catalog descriptors are never reconciled away; and the local schemaDefined declaration is never overridden, neither the live class flag nor the durable primary-key descriptor. The replicated define_schema path in Table.ts already merged additively and already passed origin: 'cluster' — this makes table() itself enforce the invariant that path assumed.

Companion PR (required to close the flake): harper-pro's ensureTableIfChanged must pass origin: 'cluster' — HarperFast/harper-pro#750 carries that one-hunk change plus the submodule bump. Without it, the guard here is unreachable from the caller that caused the incident (the cross-model review's reachability finding — correct, and answered by the pairing).

For the human reviewer

• Judgment call — additive-only vs. versioned schema: peer definitions carry no version, so "newer wins" is not implementable today; additive-only trades delete-convergence for data safety. A locally dropped attribute can be resurrected by a stale peer handshake, and a peer-side redefinition (e.g. indexed: true added remotely) is discarded by the name-only merge. Both are pre-existing properties of the handshake's honor-local policy (and of Table.ts's additive merge); the supported cluster-wide schema-change path remains deploying the schema to every node. Flagged in the review's decision ledger; a versioned schema exchange is the real long-term fix.
• Judgment call — fix location: patching harper-pro's ensureTableIfChanged alone cannot close the race, because another thread's create_table can commit its catalog writes after the caller's existence check and before table()'s reconcile. Only table() (exclusive lock + disk reload) can apply the definition safely.
• Judgment call — cluster origin skips the whole removal-reconcile scan, including removeIndex; stale index descriptors on this path persist until the next local authoring pass. Deliberately conservative.
• The updateTable emit's second argument becomes false for cluster-origin calls; no listener consumes that argument (verified by grep of all 'updateTable' registrations), so this is inert.
• Declined review suggestion — deriving index registration from the durable descriptor instead of the live attribute in the cluster-origin skip path: registration has always keyed off the caller's live attribute.indexed (the skip block mirrors the pre-existing registration lines exactly), and in both live-vs-disk disagreement directions the new path is strictly less destructive than before (no stale descriptor write, no stale reindex). Re-deriving registration from disk is a broader refactor of table()'s index handling, noted for follow-up.
• Second commit's addition: cluster-origin callers now skip every write to an existing descriptor (primary-key settings update gated; per-attribute persist/reindex skipped, index registration preserved). This closes a second-order race the harper-pro-side review found: a worker whose in-memory attribute snapshot lags a newer durable declaration would otherwise write the stale value back (and could launch a stale reindex). The new test watched this fail unguarded (a cluster-origin call rewrote a newer durable descriptor).

Verification

• New unit test unitTests/resources/clusterSchemaMerge.test.js — deterministic repro of the destructive apply: on unpatched main it fails with attribute 'name' was removed by a cluster-origin definition: id; with this patch it passes, on both engines (RocksDB and HARPER_STORAGE_ENGINE=lmdb). It also pins: peer-only attributes are still added, schemaDefined survives both live and in the durable descriptor, and local schema authoring still removes what it no longer declares.
• End-to-end route: harper-pro integrationTests/cluster/replicationLoad.test.mjs (the failing nightly suite) passes 4/4 against a build embedding this change plus the companion harper-pro change. The triggering race itself is a millisecond-wide cross-thread window (first observed after months of nightlies), evidenced by the CI server-log forensics above rather than reproduced live.
• Cross-model pre-push review ran (codex graded + gemini + cursor-composer + harper-domain): its major finding (peer schemaDefined still persisted into the durable descriptor) is fixed in the second commit with a regression assertion; the reachability major is answered by the companion PR; the delete-convergence trade is documented above.
• test:unit:resources locally: 1675 passing; 3 failures are pre-existing in this environment (identical on unpatched base — Node v26.2 / fresh-install dependency drift, none touch this change). CI is the authoritative gate.

Complexity: moderate — a guarded behavior split on an existing parameter in one heavily-shared function, plus tests; risk is concentrated in table()'s existing-table branch semantics.

_{Review-Coverage: authored=unknown; ran=none; rounds=1 @ 55afa6b}

_{Human-Review-Need: 4 @ 55afa6b}

[gemini-code-assist]

Code Review

This pull request makes cluster-origin table definitions additive-only in databases.ts to prevent peer-derived definitions from removing locally declared attributes or overriding local schema definitions. It also adds corresponding unit tests and design documentation. The review feedback recommends updating the test file to import node:assert with the node: prefix to comply with the repository's style guide.

[claude]

Reviewed; no blockers found.