Skip to content

feat(node-sdk): expose dbIntegrityCheck and checkDatabaseIntegrity - #4028

Open
insipx wants to merge 1 commit into
insipx/db-integrity-4-node-bindingsfrom
insipx/db-integrity-check
Open

feat(node-sdk): expose dbIntegrityCheck and checkDatabaseIntegrity#4028
insipx wants to merge 1 commit into
insipx/db-integrity-4-node-bindingsfrom
insipx/db-integrity-check

Conversation

@insipx

@insipx insipx commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Top of the DB-integrity-check stack (on #4032). Re-exports the node bindings' integrity API through @xmtp/node-sdk: Client.dbIntegrityCheck(level?) and the package-level checkDatabaseIntegrity / IntegrityCheckLevel / IntegrityCheckOutcome. Typecheck + eslint clean.

See #4029 for the feature overview and testing summary.

🤖 Generated with Claude Code

https://claude.ai/code/session_01V1AXHNsnQmHW174i5rRUEb

Note

Expose dbIntegrityCheck on Client and re-export checkDatabaseIntegrity from Node SDK index

  • Adds async method Client.dbIntegrityCheck(level?) in Client.ts that delegates to the underlying NodeClient instance.
  • Re-exports checkDatabaseIntegrity, IntegrityCheckLevel, and IntegrityCheckOutcome from @xmtp/node-bindings in index.ts.
  • Behavioral Change: Client.dbIntegrityCheck throws ClientNotInitializedError if called before the client is initialized.

Macroscope summarized f4a43d0.

@insipx
insipx requested review from a team as code owners August 22, 2026 14:14
@claude

claude Bot commented Aug 22, 2026

Copy link
Copy Markdown

Claude finished @insipx's task —— View job


PR Review - 2026-08-27 15:47 UTC

  • Read changed files
  • Analyze code quality and implementation
  • Check for test coverage
  • Provide feedback

Summary

This PR cleanly exposes the database integrity check functionality in the Node SDK layer. The implementation is straightforward and follows existing patterns in the codebase.

Code Quality: ✅ Good

Client.ts (lines 325-342)

  • The dbIntegrityCheck method follows the established pattern for other client methods
  • Properly throws ClientNotInitializedError when client is uninitialized (consistent with other methods)
  • JSDoc is clear and accurate
  • The parameter is correctly typed as optional with IntegrityCheckLevel type

index.ts (lines 79, 118)

  • Clean re-exports of checkDatabaseIntegrity, IntegrityCheckLevel, and IntegrityCheckOutcome
  • Alphabetically ordered within export groups (maintaining existing convention)

Test Coverage: ⚠️ Missing

Observation: While the underlying @xmtp/node-bindings package has comprehensive integrity tests (bindings/node/test/Integrity.test.ts), there are no tests at the @xmtp/node-sdk layer for:

  • Client.dbIntegrityCheck() method
  • Re-exported checkDatabaseIntegrity function

Recommendation: Add tests in sdks/js/node-sdk/test/Client.test.ts to verify:

  1. client.dbIntegrityCheck() works on a healthy database
  2. client.dbIntegrityCheck() throws ClientNotInitializedError before init
  3. Optional: Basic smoke test for the re-exported checkDatabaseIntegrity to ensure the SDK export works

This would ensure the SDK layer's API surface is tested, even though the underlying functionality is already well-tested.

Other Notes

  • No performance concerns (read-only operation)
  • No security concerns (security issues in lower layers were already addressed)
  • Type definitions are properly imported and re-exported
  • Breaking changes: None (purely additive API)

Comment thread crates/xmtp_db/src/encrypted_store/integrity.rs Outdated
@macroscopeapp

macroscopeapp Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 7bfc39c

Macroscope's review found this PR approvable — This is a small, isolated Node SDK API-surface addition that forwards to already-existing integrity-check bindings only when explicitly invoked. Existing behavior, stored data, and production configuration remain unchanged.

No code changes detected at f4a43d0. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

@insipx
insipx force-pushed the insipx/db-integrity-check branch from e121360 to 8e88dbd Compare August 22, 2026 14:19
Comment thread crates/xmtp_db/src/encrypted_store/integrity.rs Outdated
@codecov

codecov Bot commented Aug 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.87935% with 35 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.91%. Comparing base (da3e3c5) to head (e3cd796).
⚠️ Report is 14 commits behind head on main.

Files with missing lines Patch % Lines
crates/xmtp_db/src/encrypted_store/integrity.rs 93.75% 21 Missing ⚠️
crates/xmtp_db/src/encrypted_store/mod.rs 0.00% 7 Missing ⚠️
...tes/xmtp_db/src/encrypted_store/database/native.rs 85.71% 3 Missing ⚠️
crates/xmtp_mls/src/context.rs 25.00% 3 Missing ⚠️
...pted_store/database/native/sqlcipher_connection.rs 97.82% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4028      +/-   ##
==========================================
- Coverage   86.00%   85.91%   -0.09%     
==========================================
  Files         417      420       +3     
  Lines       68165    68829     +664     
==========================================
+ Hits        58622    59137     +515     
- Misses       9543     9692     +149     

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

insipx added a commit that referenced this pull request Aug 22, 2026
…and URI delimiters in paths

The salt sidecar's contents were interpolated into a PRAGMA executed
before query_only=ON; a corrupted or tampered sidecar could inject
writable SQL. Salt is now validated as exactly 32 hex chars before
interpolation. The file: URI open now percent-encodes '%', '?', '#'
so paths containing URI delimiters open the same file that was
validated.

Addresses Macroscope review feedback on #4028.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment thread crates/xmtp_db/src/encrypted_store/integrity.rs Outdated
insipx added a commit that referenced this pull request Aug 22, 2026
check_database_integrity read the salt sidecar before any existence
check, so a deleted or mistyped encrypted-DB path returned the
misleading SaltMissing instead of the documented missing-database
Failed result.

Addresses Macroscope review feedback on #4028.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@insipx
insipx force-pushed the insipx/db-integrity-check branch from e3cd796 to 2c58fbf Compare August 22, 2026 14:52
@insipx
insipx changed the base branch from main to insipx/db-integrity-4-node-bindings August 22, 2026 14:52
@insipx insipx changed the title feat: read-only SQLite database integrity checking (xmtp_db, client, node bindings) feat(node-sdk): expose dbIntegrityCheck and checkDatabaseIntegrity Aug 22, 2026
@insipx
insipx force-pushed the insipx/db-integrity-check branch from 2c58fbf to 69745ac Compare August 22, 2026 14:56
@insipx
insipx force-pushed the insipx/db-integrity-check branch from 69745ac to 82a6613 Compare August 22, 2026 15:03
@insipx
insipx force-pushed the insipx/db-integrity-check branch 2 times, most recently from ffcd028 to d2e32a6 Compare August 22, 2026 15:28
@insipx
insipx force-pushed the insipx/db-integrity-check branch from d2e32a6 to 324f9cf Compare August 25, 2026 14:13
@insipx
insipx force-pushed the insipx/db-integrity-check branch 2 times, most recently from 38ad1e4 to aff1d1c Compare August 27, 2026 14:37
@insipx
insipx force-pushed the insipx/db-integrity-check branch 2 times, most recently from 19fd1a9 to a355b22 Compare August 27, 2026 15:23
@insipx
insipx force-pushed the insipx/db-integrity-check branch from a355b22 to 7bfc39c Compare August 27, 2026 15:30
@insipx
insipx force-pushed the insipx/db-integrity-check branch from 7bfc39c to 7178f35 Compare August 27, 2026 15:38
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@insipx
insipx force-pushed the insipx/db-integrity-check branch from 7178f35 to f4a43d0 Compare August 27, 2026 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant