refactor(xmtp_mls): parse the client's own pkg_version once into LibXMTPVersion

[tylerhawkes]

Follow-up to #3864. The receive-path floor guards took the client's own pkg_version as an &str and re-parsed it into a LibXMTPVersion on every message/commit. This parses it once at VersionInfo construction and hands the typed value around.

Why

pkg_version is the client's own build identity — in production always the compile-time CARGO_PKG_VERSION (nothing in the bindings/SDKs constructs a custom VersionInfo). Re-parsing it deep in the guards was:

1. Wasted work on a hot path (parsed on every received commit/proposal).
2. A fail-unsafe silent swallow. committed_floor_exceeding did LibXMTPVersion::parse(own_version).ok()? — if our own version were unparseable it returned None → "no floor exceeds" → the below-floor pause silently never fires. Parsing once at the boundary turns "our own version is garbage" into a loud construction-time failure instead of a per-group safety hole.

What changed

• VersionInfo now caches pkg_semver: LibXMTPVersion, parsed once in the constructor and asserted valid (it's our own build/config, not remote data). New pkg_semver() accessor; pkg_version() (the &str) stays for logging/display.
• committed_floor_exceeding / _in_extensions and process_message_with_app_data take &LibXMTPVersion instead of &str.
• All receive/welcome/enforce own-version parse sites now pass context.version_info().pkg_semver(): the 3 floor-guard callers (mod.rs, validated_commit.rs, mls_sync.rs), the commit-validation enforce path (validated_commit.rs), the welcome pause (xmtp_welcome.rs), and the paused-group sweep/unpause (welcome_sync.rs, mls_sync.rs).
• LibXMTPVersion gains a Display impl (the ProtocolVersionTooLow error carries the own version as a string).
• Only the client's own version is affected. Floor / min_version / required-version values (remote data) are still parsed at the point of use and stay lenient on garbage.

Deliberately out of scope: the send-side setters (update_group_min_version / enable_proposals) that parse own version for the "min_version above own" footgun guard. Those are cold paths with their own GroupError::InvalidMinVersion handling; converting them can be a separate cleanup.

Behavior note

The old code was lenient on an unparseable own version (skip the guard). The new design makes that a panic at VersionInfo construction — which is unreachable in production (CARGO_PKG_VERSION is always valid) and is the safe direction (a garbage own-version disabling the pause is worse than failing loudly at startup). The committed_floor_exceeding_in_extensions("garbage") unit case is removed accordingly; the malformed-floor cases remain.

Test

• Floor guards, min_version monotonicity (enforce path), pause/welcome/unpause receive behaviors: 184 pass
• cargo clippy -p xmtp_mls --all-features --all-targets -Dwarnings clean on this change (see note), fmt clean

Note: main currently fails Lint (clippy needless_return_with_question_mark at mls_sync.rs:2933) and Test, from the #3864 merge — pre-existing and unrelated to this diff. This PR's Lint will inherit that trunk failure until trunk is fixed.

🤖 Generated with Claude Code

Note

Parse client pkg_version once into LibXMTPVersion at VersionInfo construction

• Adds a pkg_semver field to VersionInfo that caches the parsed LibXMTPVersion at construction time, eliminating repeated runtime parsing across the sync and welcome processing paths.
• Updates committed_floor_exceeding, process_message_with_app_data, ValidatedCommit::from_staged_commit, and unstick_paused_groups to accept or retrieve &LibXMTPVersion instead of a raw &str.
• Adds a Display impl for LibXMTPVersion so the parsed version can be formatted in error messages.
• Risk: VersionInfo::new() now panics if the supplied pkg_version string is not valid semver; previously, invalid versions would silently skip version-floor guards.

^{Macroscope summarized 0e15579.}

[claude]

Claude finished @tylerhawkes's task —— View job

---

Code Review — 2026-07-20

Summary

This PR refactors version parsing in xmtp_mls to parse the client's own pkg_version once at VersionInfo construction instead of repeatedly parsing it on every message/commit. The refactor addresses both a performance issue and a critical fail-unsafe bug where unparseable own versions silently disabled floor guards.

✅ Strengths

1. Critical Safety Fix
The old committed_floor_exceeding had:

LibXMTPVersion::parse(own_version).ok()? 

If the client's own version was unparseable, it returned None → "no floor exceeds" → the below-floor pause silently never fired. This is a security issue. The new design panics at VersionInfo construction (utils/mod.rs:111-112), which is the right fail-safe direction.

2. Performance Improvement
Eliminates redundant parsing on every message/commit in hot receive paths (7+ call sites now use pkg_semver()).

3. Clean Implementation

• VersionInfo::new() asserts validity once (utils/mod.rs:110-117)
• pkg_semver() accessor provides parsed value
• All receive-path callers updated consistently (app_data/mod.rs:253, 624; validated_commit.rs:396, 750; mls_sync.rs:485, 1167, 1257, 1436; welcome_sync.rs:290; xmtp_welcome.rs:431)

4. Display Implementation
Added Display for LibXMTPVersion (validated_commit.rs) for error messages, avoiding repetitive to_string() calls.

5. Comprehensive Testing

• Unit tests for floor guards preserved (app_data/mod.rs:809-894)
• Malformed-own-version test removed (app_data/mod.rs, line ~893 in old code) as expected — the new design makes that case unreachable via panic
• Pre-existing integration tests cover the receive paths

🔍 Minor Observations

1. Panic vs Result in VersionInfo::new()

let pkg_semver = LibXMTPVersion::parse(version)
    .unwrap_or_else(|_| panic!("client pkg_version {version:?} is not valid semver"));

Status: ✅ Appropriate. This is the client's own build identity (CARGO_PKG_VERSION), not remote data. A malformed value is a build/config bug and should fail loudly at startup, not silently break the pause mechanism. The panic is unreachable in production.

2. Send-side paths not updated
utils/mod.rs:110 comment mentions update_group_min_version / enable_proposals still parse own version.
Status: ✅ Intentional as documented in PR description — those are cold paths with their own error handling and can be cleaned up separately.

3. Test coverage for VersionInfo
The test-only test_update_version (utils/mod.rs:131-133) calls VersionInfo::new(), which will panic on invalid versions.
Status: ✅ Correct — tests should not pass garbage versions.

📊 Code Quality

• Architecture: ✅ Clean separation — parsing boundary at construction
• Type Safety: ✅ &LibXMTPVersion eliminates re-parse risk
• Error Handling: ✅ Panic for build bugs, lenient on remote data (floor values still parse at use)
• Performance: ✅ Eliminates 7+ redundant parses per commit on hot path
• Security: ✅ Critical fix — fail-unsafe silent swallow eliminated

✅ No Issues Found

• No bugs, security concerns, or performance regressions identified
• Test coverage appropriate (malformed-own-version case correctly removed)
• All call sites updated consistently
• Behavior changes are improvements (loud failure on build bugs > silent safety hole)

Conclusion

Strong approval. This PR eliminates a subtle but serious fail-unsafe bug where the pause mechanism could silently break, while simultaneously improving hot-path performance. The implementation is clean, well-tested, and the decision to panic on malformed own-version is the correct safety trade-off.

---

[macroscopeapp]

Approvability

Verdict: Approved

Mechanical refactor consolidating repeated semver parsing into a single cached parse at VersionInfo construction. All files owned by the author, no behavioral changes, just performance/code organization improvement.

^{You can customize Macroscope's approvability policy. Learn more.}

[codecov]

Codecov Report

❌ Patch coverage is 91.11111% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.64%. Comparing base (62976d0) to head (0e15579).
⚠️ Report is 10 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/xmtp_mls/src/groups/validated_commit.rs	25.00%	3 Missing ⚠️
crates/xmtp_mls/src/groups/app_data/mod.rs	95.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3876      +/-   ##
==========================================
+ Coverage   85.61%   85.64%   +0.02%     
==========================================
  Files         415      415              
  Lines       67401    67663     +262     
==========================================
+ Hits        57704    57948     +244     
- Misses       9697     9715      +18     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.