fix(xmtp_db,xmtp_mls): tolerate unknown intent kinds + app-data hardening docs

[tylerhawkes]

Problem

IntentKind's FromSql errors on unknown discriminants, and find_group_intents uses all-or-nothing load() — so a single intent row written by a newer build (e.g. a future kind 13) poisons every intent query for that group after an app downgrade: publish_intents, committed-intent processing, and sync_until_last_intent_resolved all fail, permanently wedging outbound for the group until re-upgrade. Intents are retained (not deleted) after processing, so this fires even for long-completed intents. Because v1.11's own unknown-kind behavior gets locked into the field, this tolerance can only usefully ship before the kinds it protects against exist.

Changes

• IntentKind::all() — the deserializable-kind allowlist, passed as the SQL allowed_kinds filter at the three production query sites in mls_sync. Unknown-kind rows are excluded in SQL (never deserialized), stay untouched in the table, and resume processing on re-upgrade. No Unknown sentinel variant — that would force handling in every match and risk being written back.
• Known residual (accepted, documented in the pre-v1.11 plan): find_group_intent_by_payload_hash still errors if a downgraded client meets its own newer-build commit; it holds the cursor and self-resolves on re-upgrade.
• Docs (P2.2): the floor-bump "pause, don't fork" convention added to the WELL_KNOWN change-control block in registry_table.rs; a new docs/app-data-validator-remediation.md runbook (per-group floor bumps as the no-migration remediation lever for fielded validator bugs, with the sharp edges); two stale comments removed that claimed the bootstrap receive-side validator "isn't wired" (it is — validated_commit.rs routes bootstraps through bootstrap_validator).

Testing

• intent_kind_all_is_exhaustive — pins the allowlist to every discriminant 1..=max, so adding an enum variant without updating all() fails the build's tests.
• unknown_kind_row_is_excluded_by_kind_filter — inserts a raw future-discriminant row: the kind-filtered query returns the known intent and excludes the row; the unfiltered query is pinned as still erroring (so a future behavior change is a conscious one).
• Full xmtp_db + xmtp_mls suites against the local node: 911/911 (known flake should_reconnect retried).

Part of the pre-v1.11 app-data hardening plan (P2.1 + P2.2) — the final planned code change of the set. Siblings: #3863–#3870.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PkZpajnDV829SVuW9danrW

Note

Filter unknown IntentKind discriminants to tolerate database downgrade scenarios

• Adds IntentKind::all() (via strum::EnumIter) to enumerate all known intent kinds at runtime.
• Updates sync_until_last_intent_resolved, publish_intents, and post_commit in mls_sync.rs to pass Some(IntentKind::all().collect()) as a kind filter, silently skipping rows with unrecognized discriminants inserted by a newer build.
• Adds tests in group_intent.rs verifying discriminant contiguity and that unknown-kind rows are excluded by filtered queries.
• Behavioral Change: unfiltered intent queries that previously returned all rows now error if an unknown discriminant is present; filtered queries skip those rows silently.

^{Macroscope summarized e4f44ea.}

[claude]

Claude finished @tylerhawkes's task —— View job

---

Code Review Complete

Review Date: 2026-07-21 19:56 UTC

Tasks

• Read and analyze core implementation files
• Review test coverage
• Check for potential issues not already discussed
• Provide final feedback

---

Review Summary

This PR effectively addresses the downgrade tolerance problem for IntentKind variants. The implementation is well-thought-out and the documentation is comprehensive. The approach using strum::EnumIter for exhaustiveness is solid, and the test coverage adequately verifies the critical behavior.

Positive Observations

1. Well-documented design rationale: The PR description and code comments clearly explain the problem, solution, and trade-offs
2. Comprehensive documentation: The app-data-validator-remediation.md runbook is thorough and addresses the floor-bump caveat identified in review
3. Good test coverage: Tests verify both discriminant contiguity and downgrade tolerance behavior
4. Minimal performance impact: Filtering at SQL level (vs. post-load) avoids deserialization overhead

Code Quality Observations

crates/xmtp_db/src/encrypted_store/group_intent.rs:82-101

The IntentKind::all() implementation is pragmatic. While there's a theoretical drift risk (adding a variant to the enum but forgetting the iterator), this is mitigated by:

• strum::EnumIter auto-deriving the iterator from all enum variants
• The discriminant contiguity test (intent_kind_discriminants_are_contiguous) that would catch issues
• Review visibility (iterator and match arms colocated)

The alternative (making strum a direct dependency) would be more robust but adds maintenance burden. The current trade-off is reasonable.

crates/xmtp_mls/src/groups/mls_sync.rs:584, 2830, 3954

Good: All three production query sites now use .collect() to materialize the iterator into a Vec<IntentKind>. This is necessary since allowed_kinds expects Option<Vec<IntentKind>> rather than an iterator.

Minor performance note: Each call site materializes a fresh vector. For hot paths, consider caching this as a static or OnceCell if profiling shows allocation overhead. However, these are already infrequent operations (publish/sync cycles), so the current approach is fine.

Test Coverage Assessment

crates/xmtp_db/src/encrypted_store/group_intent.rs:790-860

Excellent test coverage:

• intent_kind_discriminants_are_contiguous: Pins the discriminant layout assumption that unknown_kind_row_is_excluded_by_kind_filter depends on
• unknown_kind_row_is_excluded_by_kind_filter: Verifies the key downgrade-tolerance behavior with both filtered (production path) and unfiltered queries

The tests correctly use raw SQL insertion to simulate future variants, which is the only way to exercise this path.

Documentation Quality

docs/app-data-validator-remediation.md

The runbook is production-ready:

• Clear problem statement and remediation steps
• Addresses the "floor-bump commit is not self-pausing" caveat (lines 60-71)
• Documents sharp edges and trust model
• Good cross-references to code locations

One minor suggestion: Consider adding a worked example with version numbers and commit sequence to make the remediation steps more concrete for operators unfamiliar with the codebase.

Uncovered Edge Cases

Residual known issue (acknowledged in PR description):

• find_group_intent_by_payload_hash (group_intent.rs:563-575) still errors on unknown discriminants since it doesn't use the allowed_kinds filter
• Per the PR description, this is acceptable: a downgraded client meeting its own newer-build commit will hold the cursor and self-resolve on re-upgrade
• Consider documenting this residual at the function level (currently only mentioned in commit message and PR description)

No Blocking Issues

All previously raised concerns have been addressed. The implementation correctly balances robustness with pragmatism, and the remaining trade-offs are well-documented and intentional.

---

[macroscopeapp]

Approvability

Verdict: Approved

Defensive hardening change that filters intent queries to avoid deserialization failures during app downgrades. Uses strum::EnumIter for exhaustive variant iteration, includes comprehensive tests, and the author owns all modified code files. The unresolved review comment references stale code (the implementation now uses automatic enum iteration, not manual arrays).

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.60%. Comparing base (5001db3) to head (e4f44ea).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3871      +/-   ##
==========================================
+ Coverage   85.54%   85.60%   +0.06%     
==========================================
  Files         426      426              
  Lines       67724    67773      +49     
==========================================
+ Hits        57932    58020      +88     
+ Misses       9792     9753      -39     

☔ 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.

Dismissing prior approval to re-evaluate 2902016

[insipx]

interesting, good compile time insurance

[insipx]

good catch