Report a non-committed shared-structure save as not-saved

[kriszyp]

Summary

RecordEncoder.saveStructures (RocksDB path) wraps rootStore.transactionSync, which returns undefined when the structure-save txn was aborted (ERR_ALREADY_ABORTED is swallowed) — and the success path also fell through to undefined. msgpackr only re-packs on a === false return, so a swallowed abort looked like success: the already-encoded record (referencing the new structure id) was written even though the structure was never persisted → later Record id is not defined for N on decode (the dominant CDI rt-dev error, ~1991/10min, on local reads).

This returns true only on a committed save and false otherwise (CAS conflict or aborted txn).

Why this is the real fix for the desync

A concurrent-writer harness showed msgpackr's CAS coordination is sound; the bug is in the RocksDB save reporting. The existing retryOnBusy: true is necessary-but-insufficient (it caps at 3 busy retries, then throws; and the abort path returns undefined, never re-packing).

Pairing (must land together)

This is half the fix and depends on kriszyp/msgpackr#186: returning false alone doesn't recover, because msgpackr's re-pack reused the cached transition and re-emitted the same unpersisted-structure record. #186 marks structures uninitialized on save-failure so the re-pack reloads durable structures, rebuilds the trie, and re-mints + re-saves. Blocked on msgpackr 1.11.14 publish + dep bump.

Verified end-to-end (repro ~/dev/scripts/savestructures-abort-repro.cjs): with both fixes, a declined/aborted save → re-pack → reload + re-mint + re-save → no dangling record.

Note

Independent of the typed-structs default (#1152) — this is the classic shared-structure path, so it's needed regardless. Should cherry-pick to v5.0 (the deployed line where the error occurs).

🤖 Generated by Claude (Opus 4.7).

[gemini-code-assist]

Code Review

This pull request updates RecordEncoder.ts to explicitly return false instead of undefined when a RocksDB transaction in saveStructures fails or aborts, preventing downstream decoding issues in msgpackr. The reviewer pointed out a critical issue where this.structureUpdate is updated inside the transaction callback before the transaction is guaranteed to commit, which could lead to desyncs on aborted transactions. They provided a code suggestion to only update this state after a successful commit.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

If transactionSync aborts or fails to commit after the callback returns (e.g., due to a conflict or a busy timeout during the commit phase), the callback has already executed and set this.structureUpdate = structures. This means this.structureUpdate will remain set even though the transaction was never committed.

This can lead to downstream issues, such as the audit log incorrectly writing the HAS_STRUCTURE_UPDATE flag (lines 670-673) for a failed structure update, causing desyncs during replication or replay.

To prevent this side-effect on aborted transactions, this.structureUpdate should only be updated after transactionSync successfully commits (i.e., when committed === true). This also simplifies the redundant ternary operator.

Suggested change

	this.structureUpdate = structures;
	return true;
	},
	{ retryOnBusy: true }
	);
	return committed === true ? true : false;
	return true;
	},
	{ retryOnBusy: true }
	);
	if (committed === true) {
	this.structureUpdate = structures;
	return true;
	}
	return false;

[kriszyp]

Good catch — fixed in c7e2038. Moved this.structureUpdate = structures out of the transactionSync callback so it's only assigned when committed === true. On an aborted/declined txn it now stays unset, so no spurious HAS_STRUCTURE_UPDATE can be flagged for an unpersisted structure. Also dropped the redundant ternary. — Claude

[claude]

Reviewed; no blockers found.