Skip to content

insipx/db integrity 7 browser sdk - #4039

Closed
insipx wants to merge 1 commit into
insipx/db-integrity-6-wasm-bindingsfrom
insipx/db-integrity-7-browser-sdk
Closed

insipx/db integrity 7 browser sdk#4039
insipx wants to merge 1 commit into
insipx/db-integrity-6-wasm-bindingsfrom
insipx/db-integrity-7-browser-sdk

Conversation

@insipx

@insipx insipx commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Stacked on #4038. Browser-sdk exposure: Client.dbIntegrityCheck(level?) plumbed through the client worker RPC (action union + worker switch + WorkerClient), and Opfs.checkDatabaseIntegrity(path, level?) via the OPFS worker (the SAH pool lives in a worker context — a main-thread call would open a second pool). Types re-exported from the package barrel. Verified with real headless-chromium vitest runs against the local backend (client + by-path cases); note test/Opfs.test.ts is describe.skip upstream, so its new case rides along skipped in CI.

🤖 Generated with Claude Code

https://claude.ai/code/session_01V1AXHNsnQmHW174i5rRUEb

Note

Add database integrity check methods to browser SDK Client and Opfs

  • Adds Client.dbIntegrityCheck(level?) and Opfs.checkDatabaseIntegrity(path, level?) that run read-only checks against the underlying database and return an IntegrityCheckOutcome
  • Introduces matching worker actions "client.dbIntegrityCheck" and "opfs.checkDatabaseIntegrity" in client.ts and opfs.ts, and extends the ClientAction and OpfsAction unions accordingly
  • Re-exports IntegrityCheckOutcome and IntegrityCheckLevel types from @xmtp/wasm-bindings in index.ts
  • Adds tests covering both default and Full integrity levels for Client and Opfs

Macroscope summarized 17532ba.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@claude

claude Bot commented Aug 25, 2026

Copy link
Copy Markdown

Claude finished @insipx's task —— View job


Code Review Complete

Review Date: August 25, 2026, 14:14 UTC


Summary

This PR successfully exposes database integrity check functionality (dbIntegrityCheck and checkDatabaseIntegrity) in the browser SDK, mirroring the implementation in the Node SDK. The changes follow consistent patterns and are well-tested.

Code Quality ✅

Strengths:

  • Consistent implementation pattern across Client and Opfs classes
  • Proper TypeScript typing with IntegrityCheckLevel and IntegrityCheckOutcome
  • Follows existing architectural patterns (worker bridge pattern)
  • Clean separation of concerns between main thread and worker thread
  • Good JSDoc documentation (sdks/js/browser-sdk/src/Client.ts:1093-1097, sdks/js/browser-sdk/src/Opfs.ts:65-71)

Patterns followed correctly:

  • Worker action type definitions in sdks/js/browser-sdk/src/types/actions/client.ts:311-318 and sdks/js/browser-sdk/src/types/actions/opfs.ts:72-80
  • Worker message handling in sdks/js/browser-sdk/src/workers/client.ts:395-399 and sdks/js/browser-sdk/src/workers/opfs.ts:120-124
  • Public API exposure through index.ts with proper exports

Potential Issues 🔍

No critical issues found. Minor observations:

  1. Optional level parameter defaults: Both methods default to IntegrityCheckLevel.Quick when level is undefined, but this is handled at the WASM bindings level, not explicitly documented in the TypeScript layer. Consider adding to JSDoc for clarity.

  2. Error handling: The worker implementations rely on try-catch at the outer level (sdks/js/browser-sdk/src/workers/client.ts:1148-1154, sdks/js/browser-sdk/src/workers/opfs.ts:126-132), which is appropriate but doesn't provide specific error context for integrity check failures.

Performance Considerations ✅

Well-designed for performance:

  • Integrity checks run in Web Workers, preventing main thread blocking
  • Read-only operations (as documented) ensure no database locks
  • Async API design appropriate for potentially long-running operations
  • checkDatabaseIntegrity allows checking databases without instantiating a full client, reducing overhead

Security Concerns ✅

No security issues identified:

  • Read-only operations pose no data integrity risk
  • Path validation for OPFS operations handled by underlying WASM layer
  • No sensitive information exposure in check results
  • Worker isolation provides appropriate sandboxing

Test Coverage ✅

Comprehensive test coverage:

  • Client tests: sdks/js/browser-sdk/test/Client.test.ts:474-485 test both default and full integrity check levels
  • Opfs tests: sdks/js/browser-sdk/test/Opfs.test.ts:140-159 test integrity checks on actual client databases
  • Tests verify both "ok" outcome and empty findings arrays
  • Edge cases covered (tests create actual databases before checking)

Note: Opfs tests are currently skipped (line 7: describe.skip), but the test implementation is solid.

Recommendations

Optional improvements (not blocking):

  1. Documentation enhancement: Consider explicitly documenting the default level behavior in JSDoc:

    @param level - Check depth, defaults to `IntegrityCheckLevel.Quick` if not specified
  2. Error context: Consider enriching error messages from integrity check failures with operation context (e.g., "Database integrity check failed for path: ...")

  3. Test enablement: Enable Opfs tests once the test environment is ready (currently skipped with describe.skip)


Conclusion

This PR is ready to merge. The implementation is clean, follows established patterns, has good test coverage, and introduces no security or performance concerns. The changes successfully expose database integrity checking functionality to browser SDK users in a safe, efficient manner.


@insipx insipx closed this Aug 25, 2026
@insipx
insipx deleted the insipx/db-integrity-7-browser-sdk branch August 25, 2026 14:29
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