Skip to content

insipx/db integrity 4 node bindings - #4032

Open
insipx wants to merge 1 commit into
insipx/db-integrity-3-client-methodfrom
insipx/db-integrity-4-node-bindings
Open

insipx/db integrity 4 node bindings#4032
insipx wants to merge 1 commit into
insipx/db-integrity-3-client-methodfrom
insipx/db-integrity-4-node-bindings

Conversation

@insipx

@insipx insipx commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Stacked on #4031. Node napi surface: async checkDatabaseIntegrity(dbPath, encryptionKey?, level?) and client.dbIntegrityCheck(level?), both running the Rust work off the JS event loop via spawn_blocking, returning { outcome, findings } with outcome ∈ ok|corrupt|unreadable|saltMissing|locked|failed. Key conversion mirrors create_client. Vitest coverage: live + by-path ok on real DBs; wrong-key unreadable against a genuinely SQLCipher-encrypted DB. Full node suite 118/118.

🤖 Generated with Claude Code

https://claude.ai/code/session_01V1AXHNsnQmHW174i5rRUEb

Note

Add Node bindings for database integrity check APIs

  • Exposes dbIntegrityCheck(level?) on client.Client and a standalone check_database_integrity(path, key?, level?) function to run read-only integrity checks off the JS event loop
  • Introduces IntegrityCheckLevel enum (Quick/Full) and IntegrityCheckOutcome struct (outcome + findings) for Node-facing types
  • Standalone path-based check validates the optional 32-byte encryption key and returns a specific error on malformed input
  • Risk: check_database_integrity requires a 32-byte key; wrong-length keys return a napi::Error rather than being silently handled

Macroscope summarized b43082b.

@claude

claude Bot commented Aug 22, 2026

Copy link
Copy Markdown

Claude finished @insipx's task —— View job


PR Review Complete

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

Summary

This PR cleanly adds database integrity check APIs to the Node bindings. The implementation is solid with good error handling, proper off-event-loop execution, and adequate test coverage. Two minor improvements recommended below.


Findings

1. Inconsistent error handling between check_database_integrity and client.dbIntegrityCheck (bindings/node/src/integrity.rs:71, client/mod.rs:117)

The standalone function converts spawn_blocking join errors via .to_string(), while the client method uses Error::from_reason(e.to_string()). Both should use ErrorWrapper::from for consistency with the rest of the codebase (see create_client.rs:197, 199, 264, 267).

Current:

.map_err(|e| Error::from_reason(e.to_string()))?

Suggested:

.map_err(ErrorWrapper::from)?

2. Missing test coverage for malformed encryption key error path (bindings/node/test/Integrity.test.ts)

The standalone function validates 32-byte keys at integrity.rs:60-64, but the test suite doesn't verify that wrong-length keys return "Malformed 32 byte encryption key". The existing "wrong key" test (line 66-74) validates wrong contents, not wrong length. Consider adding:

it('rejects malformed encryption key', async () => {
  const user = createUser()
  const client = await createRegisteredClient(user)
  await client.close()
  const shortKey = new Uint8Array(16)
  await expect(checkDatabaseIntegrity(dbPathFor(user), shortKey))
    .rejects.toThrow('Malformed 32 byte encryption key')
})

Positive observations

  • Proper use of spawn_blocking to keep blocking SQLite I/O off the event loop
  • Consistent error conversion via From<IntegrityCheckResult> trait
  • Documentation correctly states ephemeral DB behavior after Macroscope feedback
  • Test coverage validates happy path, encrypted DBs, and wrong-key scenarios

@codecov

codecov Bot commented Aug 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (insipx/db-integrity-3-client-method@3adafff). Learn more about missing BASE report.

Additional details and impacted files
@@                          Coverage Diff                           @@
##             insipx/db-integrity-3-client-method    #4032   +/-   ##
======================================================================
  Coverage                                       ?   85.96%           
======================================================================
  Files                                          ?      421           
  Lines                                          ?    68882           
  Branches                                       ?        0           
======================================================================
  Hits                                           ?    59211           
  Misses                                         ?     9671           
  Partials                                       ?        0           

☔ 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
insipx force-pushed the insipx/db-integrity-4-node-bindings branch 2 times, most recently from 3ce2217 to 3febf9b Compare August 22, 2026 15:21
@insipx
insipx force-pushed the insipx/db-integrity-4-node-bindings branch from 3febf9b to 0c060df Compare August 22, 2026 15:28
@insipx
insipx marked this pull request as ready for review August 22, 2026 20:44
@insipx
insipx requested a review from a team as a code owner August 22, 2026 20:44
@macroscopeapp

macroscopeapp Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 62f5ecd

Macroscope's review found this PR approvable — Adds two explicitly invoked Node database-integrity diagnostics over existing read-only functionality, with blocking-thread execution and coverage for live, path-based, and encrypted-database checks. Existing callers, database data, schemas, and production request paths remain unchanged, and all modified files are owned by the author.

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

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

@insipx
insipx force-pushed the insipx/db-integrity-4-node-bindings branch from 0c060df to 6428c6a Compare August 25, 2026 14:13
@insipx
insipx force-pushed the insipx/db-integrity-4-node-bindings branch from 6428c6a to ddb0534 Compare August 25, 2026 14:36
@insipx
insipx force-pushed the insipx/db-integrity-4-node-bindings branch 2 times, most recently from a6a0be7 to 91830e8 Compare August 27, 2026 15:12
@insipx
insipx force-pushed the insipx/db-integrity-4-node-bindings branch from 91830e8 to 9fe25ea Compare August 27, 2026 15:23
Comment thread bindings/node/src/client/mod.rs
@insipx
insipx force-pushed the insipx/db-integrity-4-node-bindings branch from 9fe25ea to 62f5ecd Compare August 27, 2026 15:30
@insipx
insipx force-pushed the insipx/db-integrity-4-node-bindings branch from 62f5ecd to 83830bf Compare August 27, 2026 15:38
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@insipx
insipx force-pushed the insipx/db-integrity-4-node-bindings branch from 83830bf to b43082b 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