fix(universaldb): align versionstamp operand encoding

[NathanFlurry]

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[NathanFlurry]

• fix(pegboard): align drain grace defaults with stop threshold #4958 : 2 dependent PRs (#4960 , #4966 )
• fix(rivetkit): preserve bridge error messages #4959
• fix(rivetkit): fire abort signal on shutdown finalize #4957
• fix(rivetkit): use startup kv preload #4956
• fix(client): flatten skipReadyWait option #4955
• ci(publish): add manual preview publish flow #4954
• fix(depot): add ltx decode diagnostics #4953
• fix(universaldb): preserve plain set values #4952
• fix(universaldb): align versionstamp operand encoding #4951 👈 (View in Graphite)
• fix(rivetkit): export database provider types #4950
• wip: kitchen sink agentic loop finish #4949
• chore(rivetkit): note duplicate admission logic #4948
• test(rivetkit): cover bypass fetch header forwarding #4947
• feat(kitchen-sink): add actor probe controls #4946
• fix(gateway): split actor response wait errors #4945
• fix(guard): handle bypass preflight before query parse #4944
• feat(kitchen-sink): add mock agent event log copy #4943
• fix(kitchen-sink): default mock agent to local endpoint #4942
• test(rivetkit): enable driver lifecycle regressions #4941
• test(rivetkit): verify cached websocket db refs exceed grace #4940
• fix(rivetkit-core): keep sleeping actors reachable during grace #4939
• test(rivetkit): cover async websocket close db access #4938
• fix(rivetkit): keep actors awake until keepAwake work finishes #4937
• fix(rivetkit): persist native onSleep state on errors #4936
• fix(rivetkit-core): allow dispatch during sleep grace #4935
• fix(rivetkit): make duplicate sleep requests idempotent #4934
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

PR #4951: fix(universaldb): align versionstamp operand encoding

Overview

This PR fixes two related bugs in the universaldb versionstamp substitution logic:

1. Wrong versionstamp write size -- the old code used VERSIONSTAMP_SIZE = 12 and copied all 12 bytes of the internal Versionstamp representation, overwriting the 2-byte user version field that belongs to the tuple encoding. The correct FDB versionstamp write size is 10 bytes.
2. Offset trailer not stripped -- the 4-byte operand offset appended for FDB SET_VERSIONSTAMPED_* operations was read via split_off() (mutating in place) but never stripped from the stored result. The fix instead reads the offset as a slice and then calls truncate() after substitution.

The semantic correction in test_versionstamp_incomplete_preserves_user_version is also important: the assertion changes from "substitution replaces user_version" to "substitution preserves user_version". The user version embedded in Versionstamp::incomplete(uv) is part of the tuple key and must survive substitution.

What's correct

• VERSIONSTAMP_SIZE 12 to 10 -- FDB versionstamps are 10 bytes. Bytes 10-11 of the 12-byte Rust struct hold the user version for tuple encoding and must not be overwritten. The fix is correct.
• split_off() to slice + truncate() -- switching from destructive split_off to a non-mutating slice read with a deferred truncate is cleaner and correct in all paths.
• truncate on the already-complete path -- the old code returned early without stripping the offset trailer when the versionstamp was already complete. The fix correctly strips it in both paths.
• Test additions -- the four new tests cover: explicit operand trimming, FDB metadata-value pattern (offset=0), depot suffix preservation, and cross-validation against the tuple's own offset. These directly encode FDB wire protocol invariants.

Issues / concerns

1. substitute_versionstamp_if_incomplete has no test coverage

The heuristic helper at the bottom of versionstamp.rs calls substitute_versionstamp internally so the fix flows through, but the helper itself is untested. It applies a marker sniff-and-substitute heuristic on arbitrary bytes, which is inherently fragile. A false-positive match could silently corrupt a value. A test (or at least a comment documenting expected callers and invariants) would help.

2. Semantics mismatch between the two substitution functions

• substitute_versionstamp skips substitution if the target bytes are not [0xff; 10] (idempotent already-complete guard).
• substitute_raw_versionstamp has no such guard and unconditionally overwrites whatever bytes are at the given offset.

This is visible in test_substitute_raw_versionstamp_matches_fdb_metadata_value_operand, which starts with 10 zero bytes and expects them to be overwritten. The intentional difference is worth a comment since callers need to know which function provides idempotency.

3. test_substitute_versionstamp_already_complete tests fragile state

The test manually re-appends 1u32.to_le_bytes() as an offset after the first substitution has already truncated the vector. The re-appended offset=1 points to the second byte of the packed tuple, which may or may not have a 0x33 marker depending on packed content. The test only checks result.is_ok() without verifying the bytes were left unchanged. Consider asserting the versionstamp bytes are unmodified.

4. Duplicate magic constant

VERSIONSTAMP_SIZE in substitute_versionstamp and the local versionstamp_len = 10 in substitute_raw_versionstamp refer to the same concept (raw FDB versionstamp byte width). A shared module-level constant would prevent drift.

Minor nits

• The bounds check error message says "exceeds" but the predicate is >= (includes ==). Fine in practice since offset == data_len is equally invalid, but "equals or exceeds" would be more precise.

Summary

The core fix is correct and necessary -- the old code was actively wrong for call sites relying on user-version preservation or expecting the 4-byte offset trailer to be stripped from stored data. The test additions directly encode FDB wire-protocol invariants and will catch future regressions. Main items to address before merging: (1) add a test or documented invariant for substitute_versionstamp_if_incomplete, and (2) add a comment explaining why substitute_raw_versionstamp intentionally does not guard against already-complete versionstamps.