feat: Add Rust FFI client for integration testing by jackzampolin · Pull Request #4422 · sourcenetwork/defradb

jackzampolin · 2026-01-26T22:59:26Z

Summary

Adds a new rust-ffi client type to the Go integration test framework, enabling DefraDB's 2800+ integration tests to run against the Rust FFI implementation. This is the core compatibility testing infrastructure for defradb.rs.

What This PR Does

The Rust implementation of DefraDB exposes a C FFI. This PR adds a Go wrapper (tests/clients/rustffi/) that implements the same client.TxnStore interface as the Go, HTTP, CLI, and JS clients, allowing the entire integration test suite to run against Rust via DEFRA_CLIENT_RUST_FFI=true.

Current coverage: 2725 passing / 2817 total (96%) across 102 test packages.

Key Components

New Files

tests/clients/rustffi/defra.go — CGO bindings to the Rust FFI C API (node lifecycle, CRUD, P2P, ACP, transactions, subscriptions)
tests/clients/rustffi/wrapper.go — Implements client.TxnStore interface, translating Go test operations into FFI calls with JSON/GraphQL serialization
tests/clients/rustffi/defra.h — C header generated from the Rust cbindgen output
tests/clients/rustffi/identity.go — Identity registration bridge for block signing
scripts/run-rust-ffi-tests.sh — Per-package test runner with JSON report generation
scripts/list-test-packages.sh — Lists all integration test packages
Makefile additions — test:ffi and test:ffi:all targets

Modified Files (non-trivial)

tests/state/ — Added RustFFIClientType, build-tagged identity registration hooks (identity_rust.go / identity_norust.go)
tests/action/ — Extended SyncBranchableCollection with FFI-specific merge event tracking, added PeerInfo action
tests/integration/events.go — Renamed event wait functions to match P2P API renames from develop
cbindings/ — Extended C bindings with identity signing, P2P methods, lens operations
client/document.go — Added SetDocID() (for FFI docID sync after Rust applies defaults), added read lock to ID()
internal/encoding/int.go — Fixed int type decoding for cross-language compatibility
tests/integration/acp/nac/ — Enabled NAC tests for the Rust FFI client

Merged from develop

This branch includes several merged PRs that landed on develop during development:

P2P API renames: replicator set→create, getall→list, document add→create, remove→delete (refactor: Make P2P document names consistent #4469, chore: Make P2P replicator names consistent #4472)
NAC gating for PeerInfo and lens list (feat: Gate PeerInfo with NAC & Add Context #4282, feat: Gate lens list op with NAC #4491)
Int type decoding fix (fix(i): Document decode int types changes #4466)

Architecture

Go Test Suite
    │
    ├── GoClientType      → direct Go API calls
    ├── HTTPClientType    → HTTP API calls  
    ├── CLIClientType     → CLI subprocess
    ├── JSClientType      → JavaScript bindings
    └── RustFFIClientType → CGO → Rust C FFI  ← NEW
                                    │
                                    ▼
                              defradb.rs (Rust)

The FFI wrapper bridges Go↔Rust by:

Serializing Go structs to JSON/GraphQL
Calling C FFI functions via CGO
Deserializing Rust JSON responses back to Go types
Bridging Rust events (merge complete, replicator, topic peer) to Go's event bus via a polling goroutine

How to Run

Tests are best run from the Rust side using the ffi-test CLI tool, which manages paired Rust+Go worktrees, builds the FFI library, and generates JSON test reports:

# Install the tool (once, from the defradb.rs repo)
cargo install --path tools/ffi-test

# Run a specific test package
ffi-test run query/simple

# Run all sub-packages under a prefix
ffi-test run query

# Run a specific test by name pattern
ffi-test run query/simple -t TestQuerySimple

# Skip rebuilding the Rust library (faster iteration on Go-side changes)
ffi-test run query/simple --skip-build

# View current compatibility status (current worktree)
ffi-test status

# View status across all worktrees
ffi-test status --all

The Makefile targets also work for manual invocation:

make test:ffi RUST_LIB=/path/to/defradb.rs FFI_PKG=query/simple
make test:ffi:all RUST_LIB=/path/to/defradb.rs

Both require the Rust FFI shared library (libdefra_ffi.dylib / .so) built from defradb.rs.

Remaining Failures & Skips

Failures (4 tests)

Package	Test	Issue	Root Cause
mutation	`TestTransactionalCreationAndLinkingOfRelationalDocumentsForward`	defradb.rs#382	One-to-one relation uniqueness not enforced across concurrent txns
net/simple	`TestP2POneToOneReplicatorDoesNotSyncFromDeletedReplicator`	defradb.rs#305	Replicator deletion doesn't stop in-flight syncs
net/sync	`TestDocSync_WithDifferentVersionsOnPeers_ShouldSyncLatest`	defradb.rs#305	Merge event timeout during multi-version sync
node	`TestNodeIdentity_NodeIdentity_Succeed`	defradb.rs#306	Node identity not persisted after configuration

Skips (88 tests)

Most skips are Go test harness gating, not missing Rust functionality. Full analysis: defradb.rs#317

~48: SourceHub ENV-gated (require external infrastructure, pass when ENV is set)
~12: Need DEFRA_MUTATION_TYPE=gql second pass
~8: Need persistent storage wiring in Go wrapper
~6: Encrypted index SupportedClientTypes update needed
~5: CRDT counter overflow behavior divergence
~4: Embedding generation (needs Ollama, deferred)
~2: ed25519 key support needed
~3: View materialization stubs

Overall tracking: defradb.rs#382 — FFI Compatibility Status

Test Plan

All existing Go integration tests pass (no regressions on develop client types)
96% FFI test coverage (2725/2817 tests passing)
Lint clean (forbidigo, errcheck, lll)
Remaining 4 FFI failures tracked in Rust implementation (defradb.rs#305, defradb.rs#306, defradb.rs#382)
Skip reduction tracked in defradb.rs#317

🤖 Generated with Claude Code

Add support for running Go integration tests against the Rust FFI implementation of DefraDB. This enables using Go's 600+ integration tests as a compatibility test suite for the Rust implementation. Changes: - Add RustFFIClientType to test state - Add DEFRA_CLIENT_RUST_FFI env var support - Create tests/clients/rustffi/ with CGO bindings to Rust FFI - Add scripts for running tests and generating failure reports Usage: DEFRA_CLIENT_RUST_FFI=true go test ./tests/integration/query/simple/... Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

coderabbitai · 2026-01-26T22:59:35Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds a Rust FFI client and full testing pipeline: C header, Go CGO bindings and wrapper, test-framework integration, Makefile targets and scripts for running Go integration tests against the Rust FFI, plus broad expansion of tests to include the new client type.

Changes

Cohort / File(s)	Summary
FFI C API & Go CGO bindings `tests/clients/rustffi/defra.h`, `tests/clients/rustffi/defra.go`	Adds a large C FFI header and corresponding Go CGO bindings exposing node lifecycle, queries, transactions, NAC/DAC, P2P, subscriptions, indexes, import/export, and many helpers.
Rust FFI Go wrapper & helpers `tests/clients/rustffi/wrapper.go`, `tests/clients/rustffi/identity.go`	Implements a Go Wrapper around the Rust node (Wrapper, TxnWrapper, CollectionWrapper), event bridging, identity registration helper, and extensive store/P2P API surface used by tests.
Build, orchestration & reporting `Makefile`, `scripts/run-rust-ffi-tests.sh`, `scripts/list-test-packages.sh`, `.claude/skills/rust-ffi-compat.skill`, `.gitignore`	Adds Makefile targets for FFI test pipelines (per-package GOCACHE, metadata, history/reporting), runner scripts to invoke go test with CGO env and produce Markdown reports, package listing script, and .gitignore patterns for reports/artifacts.
Test framework wiring & client setup `tests/integration/client.go`, `tests/integration/client_setup.go`, `tests/integration/client_setup_rustffi.go`, `tests/integration/client_setup_norustffi.go`	Detects DEFRA_CLIENT_RUST_FFI, extends setupClient signature, adds setupRustFFIClient implementation (and non-FFI stub) that initializes wrapper, mirrors NAC/SE state, forwards events, and configures SourceHub when present.
Tests: add Rust FFI client type & state `tests/state/client_type.go`, `tests/state/state.go`, `tests/state/identity*.go`, `tests/state/identity_rust.go`	Adds `RustFFIClientType`, new State fields for SourceHub endpoints and signer key, and build-tag conditional helpers to register identities with Rust when using the FFI client.
Test assertions, results & helpers `tests/action/*.go`, `tests/integration/results.go`, `tests/integration/explain_result_asserter.go`, `tests/integration/events.go`	Extends result/assertion logic to include RustFFIClientType, improves numeric handling, populates ExpectedDAGHeads from peers, and adds logging/debug info for sync flows.
Test matrix expansions `tests/integration/acp/nac/*/.go`, many test files under `tests/integration/` and `tests/integration/acp/relation_admin/`	Adds RustFFIClientType to ~25+ NAC, signature, P2P, query, and other integration tests to exercise the new client across the suite.
Client-facing small changes `cbindings/errors.go`, `cbindings/wrapper_tx.go`, `client/document.go`	Adds FFI-specific error constructor `NewErrFFI`, delegates some tx methods to Go-native txn for context preservation, and exposes `Document.SetDocID` for external ID setting.
P2P retry wiring `tests/integration/p2p.go`	Adds `replicatorRetrier` interface and triggers `RetryReplicators()` for clients that expose it after peer reconnection (addresses Rust wrapper lacking background retry).

Sequence Diagram(s)

sequenceDiagram
    participant Test as Test
    participant Wrapper as RustFFI Wrapper
    participant RustNode as Rust FFI Node
    participant EventBus as Go EventBus

    Test->>Wrapper: setupRustFFIClient(identity, config)
    Wrapper->>RustNode: new_node / new_node_with_p2p(opts)
    RustNode-->>Wrapper: node_ptr

    alt Mirror NAC state
        Wrapper->>Test: read Go NAC state
        Wrapper->>RustNode: enable_nac / add_actor_relationships
    end

    Wrapper->>EventBus: ForwardSEEvents(goBus)
    Wrapper-->>Test: ready

    Test->>Wrapper: ExecRequest(query)
    Wrapper->>RustNode: exec_request(...)
    RustNode-->>Wrapper: result JSON
    Wrapper->>EventBus: Publish(events)
    EventBus-->>Test: deliver events

sequenceDiagram
    participant Makefile as Makefile Target
    participant Script as run-rust-ffi-tests.sh
    participant GoTest as go test
    participant RustLib as Rust FFI lib
    participant Report as Markdown Report

    Makefile->>Makefile: validate RUST_LIB / generate defra.h
    Makefile->>Makefile: copy library
    loop per-package
        Makefile->>Script: invoke PACKAGE, OUTPUT_DIR, GOCACHE
        Script->>GoTest: run go test -C package (CGO flags)
        GoTest->>RustLib: call FFI APIs
        RustLib-->>GoTest: responses
        GoTest-->>Script: stdout/stderr
        Script->>Report: parse RUN/PASS/FAIL and failures
        Script-->>Report: write package report
    end
    Makefile->>Makefile: aggregate results, save history

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

feat: Add Makefile target for FFI test runner #4450: Implements the Makefile targets, per-package caching, reporting and runner scripts described in the issue.

Possibly related PRs

fix: Flaky SourceHub tests #4416: Related changes to SourceHub test setup and endpoint/state handling.
test: Migrate request test actions to new framework #4359: Overlaps test action/assertion changes and client-type handling.
feat: Gate PeerInfo with NAC & Add Context #4282: Related P2P/PeerInfo/NAC context changes that intersect with FFI client integration.

Suggested labels

feature, area/testing

Suggested reviewers

fredcarle
AndrewSisley
shahzadlone

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2026-01-26T23:09:56Z

Codecov Report

❌ Patch coverage is 25.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.50%. Comparing base (b40ba72) to head (921e0ba).
⚠️ Report is 2 commits behind head on develop.

Files with missing lines	Patch %	Lines
cbindings/errors.go	0.00%	2 Missing ⚠️
cbindings/wrapper_tx.go	0.00%	2 Missing ⚠️
client/document.go	50.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4422      +/-   ##
===========================================
- Coverage    75.63%   71.50%   -4.12%     
===========================================
  Files          514      514              
  Lines        40653    40666      +13     
===========================================
- Hits         30744    29077    -1667     
- Misses        7452     9206    +1754     
+ Partials      2457     2383      -74

Flag	Coverage Δ
all-tests	`71.50% <25.00%> (-4.12%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
cbindings/errors.go	`16.67% <0.00%> (-23.33%)`	⬇️
cbindings/wrapper_tx.go	`0.00% <0.00%> (-16.46%)`	⬇️
client/document.go	`70.38% <50.00%> (-0.14%)`	⬇️

... and 84 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- Fix Save() to check document existence before deciding create vs update - Add event publishing after Create and Update mutations - Implement proper event bus with subscription filtering by event name - Convert JSON to GraphQL input format (unquoted keys) - Add RustFFIClientType to relaxed json.Number comparison in test framework Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Remove hardcoded paths and allow users to set DEFRA_RS_PATH and configure CGO flags via environment variables: export DEFRA_RS_PATH=/path/to/defradb.rs export CGO_CFLAGS="-I$DEFRA_RS_PATH/crates/ffi" export CGO_LDFLAGS="-L$DEFRA_RS_PATH/target/release" Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Scripts now require DEFRA_RS_PATH to be set and will validate it exists before running tests. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Skill enables quick generation of compatibility reports by running Go integration tests against the Rust FFI implementation. Usage: /rust-ffi-compat or ask about "rust compatibility status" Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Run all tests so we can track complete progress. Let them fail if they fail rather than skipping. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Two fixes that improve FFI test compatibility: 1. Use json.Decoder.UseNumber() in wrapper.go to preserve JSON numbers as json.Number instead of converting to float64. This allows proper int64/float64 distinction during test assertions. 2. Add RustFFIClientType to isResultsEqual switch case in assert_request.go so non-ordered result comparisons use the relaxed comparison logic (areResultsEqual) that handles json.Number. These changes fix 22 additional tests (161 -> 183 passing). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Wire up the Rust FFI add_dac_policy function to Go: - Add AddDACPolicy method to Node in defra.go - Update wrapper.go to call FFI instead of stub error - Update defra.h with function declarations from Rust crate Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Wire up P2P functionality in the Go FFI wrapper: - Add Go FFI bindings for 10 P2P functions (NewNodeWithP2P, P2PPeerInfo, P2PActivePeers, P2PConnect, P2PSetReplicator, P2PDeleteReplicator, P2PGetAllReplicators, P2PAddCollections, P2PRemoveCollections, P2PGetAllCollections) - Update wrapper.go to call FFI functions instead of returning stubs - Update defra.h with P2P function declarations from Rust crate This enables P2P tests to run against the Rust FFI implementation. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

- Add NewWrapperWithP2P function to wrapper.go for P2P-enabled nodes - Update client_setup.go to use NewWrapperWithP2P when IsNetworkEnabled - Add RustFFIClientType to SupportedClientTypes in P2P test files (18 files) - Update defra.h with P2P function declarations This allows P2P tests to run against the Rust FFI implementation. P2P is now initializing (libp2p host created), though full test compatibility may need additional work. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

- Fix double-counting bug in run-rust-ffi-tests.sh (count only === RUN lines) - Add reports/ and Rust library files to .gitignore Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

…kflow - Update defra.go to use ${SRCDIR} for header include only - Remove -L${SRCDIR} from LDFLAGS; library path now comes from CGO_LDFLAGS - Update defra.h to latest version with P2P function declarations - Update skill file with new test invocation pattern This enables a single Go repo to test against multiple Rust worktrees by pointing CGO_LDFLAGS at each worktree's target/release/ directory. No library copying required. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

- Emphasize inline env vars with go test -C pattern - Warn against cd, export, and wrapper scripts - Add verification step to check client=rust-ffi in output - Add troubleshooting entries for common shell errors Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

For move/copy operations, use the "from" path to determine the source collection rather than the "path" field. This ensures the patch is routed to the correct collection when moving/copying between collections. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

…ting" This reverts commit c439830.

Changes: - Fix UpdateWithFilter to properly validate filter and updater parameters - Add string filter handling without JSON quote wrapping - Add convertDateTimeStrings() to convert RFC3339 strings to time.Time - Add transaction context handling in ExecRequest These changes work with the Rust FFI fixes to improve mutation/update test parity from 74% to 84%. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

- Update Get() to query ALL scalar fields (not just _docID) so SetWithJSON can properly track dirty/modified fields - Skip ALL relation fields (both single objects and arrays) to avoid "cannot set relation from secondary side" errors - Add json.Number handling in convertDateTimeStrings to convert numeric values to int64/float64 as appropriate - Add checkIfDocumentDeleted helper to detect deleted documents and return appropriate error messages This fixes TestMutationUpdate_WithDefaultValues_DoesNotOverwrite and TestMutationUpdateOneToMany_RelationIDToLinkFromSingleSide_Error. Test status: 83/98 passing (85%), up from 73/98 (74%) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

The regex-based approach incorrectly matched escaped quotes inside string values, mangling JSON strings like "{\"one\":1}" into "{\one\:1}". Replace with a proper JSON parser approach that: - Parses JSON into Go values - Recursively rebuilds GraphQL input syntax - Properly escapes string values without modifying their content Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

…elete() - Add isDocumentDeleted helper to check if doc was soft-deleted - Update Save() to check for deleted docs before creating - Update Delete() to publish update events (matches Go DefraDB behavior) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Add test:ffi, test:ffi-all, and test:ffi-report targets to simplify running Go integration tests against the Rust FFI implementation. Features: - Automatic GOCACHE clearing (avoids stale CGO objects) - Run once, parse multiple views (no redundant test runs) - Pass/fail summary with failed test list - Report command to view results across packages Usage: make test:ffi RUST_LIB=/path/to/defradb.rs PKG=query/simple make test:ffi-all RUST_LIB=/path/to/defradb.rs make test:ffi-report Closes #4450 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Add complete list of integration test packages to Makefile, organized by priority levels from tracking issue #18: - P0 Foundation: query/simple, mutation/* - P1 Core Query: inline_array, commits, json - P2 Relations: one_to_*, many_to_many (10 packages) - P3 Features: explain, acp, index, versions, etc. (14 packages) New targets: - test:ffi-p0 - Run P0 foundation tests only - test:ffi-p1 - Run P1 core query tests only - test:ffi-p2 - Run P2 relation tests only - test:ffi-p3 - Run P3 feature tests only - test:ffi-all - Run all 34 test packages Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Track test results over time with metadata about which Rust branch/commit produced them. History is stored in ffi-test-history/ (gitignored). New commands: - make test:ffi-save RUST_LIB=... - Save current reports to history - make test:ffi-history - View past test runs (last 20) History entries include: - Timestamp - Rust repo path, branch, and commit - Pass/fail counts for all packages Also adds TOTAL row to test:ffi-report output. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

grep -c returns exit code 1 when no matches found, even though it outputs '0'. Using '|| echo 0' caused double output. Fixed by using '; true' to suppress exit code while keeping grep's output. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Display the Rust branch, commit, worktree name, and timestamp at the top of test:ffi-report output so it's clear which build produced the results. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Each package now stores its own .meta file alongside the .log with the Rust branch, commit, and timestamp of the run that produced it. This supports mixed-worktree test runs where different packages may come from different Rust builds at different times. Report output now shows per-line metadata: defradb.rs-query-simple 410/433 (94%) main@ce0128f 2026-01-29 09:42 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Regenerated from defradb.rs main@83dd36f (PR #248 ffi-complete merge). Removes NewTxnResult struct, updates comment formatting. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Runs cbindgen against the target Rust worktree at the start of test:ffi, so the C header always matches the code being tested. Eliminates stale header bugs when FFI signatures change. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Update the defra.h header comments to reflect the actual implementation: - Clarify that only materialized views are refreshed - Fix the options parameter description - Remove "Not yet implemented" note Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Brings in FFI binding updates and test fixes for materialized view caching. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

- Add goNodeCloser to Wrapper to release Go node's badger lock on close - Wire Go node closer in setupRustFFIClient for restart test support - Skip NAC admin writes when NAC is in disabled state (already persisted) - Guard DisableNAC mirroring when Rust node already loaded disabled state Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Pass SourceHub connection info (REST address, CometBFT RPC address, chain ID, signer key) from the Go test harness through FFI to the Rust node. Enable the REST/LCD API server on a unique port per test instance so the Rust client can query SourceHub. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

1. Remove duplicate event publishing from CollectionWrapper.Create(), Update(), and Delete() — ExecRequest.emitMutationEvents already handles this. Double emission poisoned the event channel causing "unexpected document update" errors in relation/aggregate/index tests. 2. Add empty-result detection in CollectionWrapper.Delete() for collections with ACP policies. Returns "document not found or not authorized to access" when the delete mutation returns no results. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Update auto-generated defra.h to reflect new subscription FFI functions and updated doc comments for poll/close_graphql_subscription. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

CWrapper loses transaction context for these operations because it goes through FFI which uses context.Background(). Delegating to the inner Go-native txn preserves uncommitted writes and transaction context. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…h merge Brings back txn-aware GetCollections, batch version deletion, and PatchCollection rewrite for remove/copy operations from the ffi/collection-version branch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Resolve conflicts from P2P method renames (Add->Create, Remove->Delete, GetAll->List), setupNode signature simplification, and SourceHub testcontainers migration. Update Rust FFI wrapper to match new interfaces. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Update defra.go to use renamed Rust FFI symbols: p2p_create_replicator, p2p_list_replicators, p2p_create/delete/list_collections, p2p_create/delete/list_documents. Add identity_did param to p2p_peer_info and lens_list calls. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

coderabbitai

Actionable comments posted: 12

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

🤖 Fix all issues with AI agents

In `@client/document.go`:
- Around line 973-977: ID() currently reads doc.id without synchronization while
SetDocID and GenerateAndSetDocID mutate doc.id under a lock, which creates a
potential data race; to fix, update the ID() method to acquire the document's
read lock (e.g., use doc.mu.RLock()/RUnlock()) around the access to doc.id so
reads are synchronized with SetDocID/GenerateAndSetDocID, or alternatively add
clear concurrency docs on the public mutators explaining callers must
synchronize—reference the ID(), SetDocID, GenerateAndSetDocID methods and the
doc.id field when making the change.

In `@Makefile`:
- Around line 514-515: Remove the duplicated echo invocation; locate the two
identical lines '@echo "=== Generating defra.h from $(RUST_LIB) ==="' in the
Makefile and delete one of them so only a single echo remains, leaving the
generation message printed once.

In `@scripts/run-rust-ffi-tests.sh`:
- Around line 46-48: The three count assignments (TOTAL, PASSED, FAILED) use
"grep -c ... || echo 0" which can emit duplicate zeros when grep returns no
matches; change the fallbacks to "|| true" so the command substitution only
captures grep's own "0" output (e.g., update the lines assigning TOTAL, PASSED,
and FAILED to use "grep -c ... || true"), keeping the same grep patterns and
variable names.

In `@tests/action/sync_branchable_collection.go`:
- Around line 62-77: The code reads a.s.DocIDs[a.CollectionID] without ensuring
the index exists, which can panic if DocIDs has fewer entries than the
CollectionID; update the loop to safely obtain docIDs by checking that
a.CollectionID is within bounds of a.s.DocIDs (e.g., len(a.s.DocIDs) >
a.CollectionID) and skip the per-peer copy when missing or when docIDs is
nil/empty; keep the rest of the logic (iterating docIDs, using
peerNode.P2P.ActualDAGHeads and setting nodeState.P2P.ExpectedDAGHeads)
unchanged and reference these symbols (a.s.DocIDs, a.CollectionID,
nodeState.Collections, peerNode.P2P.ActualDAGHeads,
nodeState.P2P.ExpectedDAGHeads) when applying the guard.

In `@tests/clients/rustffi/defra.go`:
- Around line 34-40: Remove the unused sentinel error ErrNotInitialized (or use
it): either delete the declaration ErrNotInitialized from the var block in
defra.go, or update Init and any FFI entry points to return ErrNotInitialized
when called before initialization (use the existing initOnce and Init
function/logic to gate calls). Ensure references use the exact symbol name
ErrNotInitialized and that Init and any public FFI functions check
initialization state consistently.
- Line 1878: The call json.Marshal(collections) currently ignores its error
(collectionsJSON, _ := json.Marshal(collections)); update this to capture and
handle the error like the other marshal calls in this file — e.g.,
collectionsJSON, err := json.Marshal(collections) and then check if err != nil
and fail/return (use the same error-handling pattern used nearby, such as
t.Fatalf/log.Fatalf/return err) so failures aren’t silently ignored; reference
the collectionsJSON variable and the json.Marshal(collections) invocation when
making the change.

In `@tests/clients/rustffi/wrapper.go`:
- Around line 236-266: Replace development prints (fmt.Printf / fmt.Println)
inside the event poller loop with structured logger calls at Debug level (or
remove them); specifically replace the fmt.Printf/fmt.Println usages that log
MergeComplete (mc, mc.Merge.DocID, mc.Merge.Cid), replicator_completed,
topic_peer_event (result.Event.PeerID, result.Event.Topic,
result.Event.EventType), and se_artifact_received (result.Event.DocID) with
calls to your logger (not fmt) before calling eb.Publish(event.NewMessage(...));
locate these by looking for fmt.Printf/fmt.Println near usages of eb.Publish,
event.MergeCompleteName, event.ReplicatorCompletedName,
event.TopicPeerEventName, and event.SEArtifactReceivedName and convert them to
debug-level structured logs.
- Around line 595-597: The call to CloseGraphQLSubscription in
Wrapper.pollGraphQLSubscription currently returns an error that is not handled
which trips errcheck; explicitly discard the return value to silence the linter.
Update the defer that calls CloseGraphQLSubscription(subscriptionID) inside the
pollGraphQLSubscription method to assign its return to the blank identifier
(e.g. _ = CloseGraphQLSubscription(subscriptionID)) so the error is
intentionally ignored.
- Around line 359-380: The Close method on Wrapper currently ignores the error
returned by w.node.Close(); update Wrapper.Close to capture the return value
from w.node.Close() and either handle it (e.g., log the error using the
project's logger or fmt) or explicitly discard it (assign to the blank
identifier) so the compiler warning is resolved; specifically modify the
w.node.Close() call inside Wrapper.Close to check its error and act accordingly.
- Around line 1836-1905: The eventBus has data races on the closed flag and subs
slice; add a sync.RWMutex field (e.g., mu) to the eventBus struct and use
mu.RLock()/mu.RUnlock() in Publish when reading e.closed and iterating e.subs
(around wantsEvent/send logic), and use mu.Lock()/mu.Unlock() in Subscribe,
Unsubscribe, and Close when appending/removing from e.subs or setting e.closed;
keep the recover() as a defensive measure but rely on the mutex to prevent
concurrent access races on eventBus, referencing the eventBus type and its
methods Publish, Subscribe, Unsubscribe, and Close and the fields closed and
subs.
- Around line 194-225: The merge-poller goroutine currently ignores Close()
errors and uses forbidden fmt prints; update it to (1) handle or explicitly
discard the error returns from node.Close() and mergeSub.Close() (referencing
node.Close() and mergeSub.Close()) instead of leaving them unchecked, and (2)
replace all fmt.Println/fmt.Printf usages inside the goroutine (the startup,
stop, poll error, subscription closed, event log, CID decode error, etc., around
the merge poller that calls mergeSub.Poll() and inspects result) with the
project's structured logger (e.g., corelog or slog) so messages are logged
properly and lint-compliant; ensure logs include the same contextual fields
(event.Type, DocID, CID, CollectionID, ByPeer) and propagate or log errors from
CID decode and Poll instead of printing to stdout.

In `@tests/state/identity.go`:
- Line 25: The tests/state package unconditionally imports cbindings causing
non-CGO builds to fail; change to use a hook instead: add a RegisterIdentityHook
func(acpIdentity.Identity) error field (or similar) on the state struct in
tests/state/state.go and replace direct calls to
cbindings.RegisterIdentityWithRust in tests/state/identity.go with calls to
s.RegisterIdentityHook (guarded by nil). Then set s.RegisterIdentityHook =
cbindings.RegisterIdentityWithRust from the Rust-FFI-specific code (the file
that already imports cbindings) or provide a CGO build-tagged initializer so
non-CGO builds never import cbindings.

🧹 Nitpick comments (18)

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b40ba72 and 4b2b6a7.

📒 Files selected for processing (60)

.claude/skills/rust-ffi-compat.skill
.gitignore
Makefile
cbindings/errors.go
cbindings/identity.go
cbindings/wrapper_tx.go
client/document.go
scripts/list-test-packages.sh
scripts/run-rust-ffi-tests.sh
tests/action/assert_request.go
tests/action/results.go
tests/action/sync_branchable_collection.go
tests/clients/rustffi/defra.go
tests/clients/rustffi/defra.h
tests/clients/rustffi/wrapper.go
tests/integration/acp/nac/index_create_test.go
tests/integration/acp/nac/index_drop_test.go
tests/integration/acp/nac/index_list_test.go
tests/integration/acp/nac/p2p_collection_create_test.go
tests/integration/acp/nac/p2p_collection_delete_test.go
tests/integration/acp/nac/p2p_collection_list_test.go
tests/integration/acp/nac/p2p_document_create_test.go
tests/integration/acp/nac/p2p_document_delete_test.go
tests/integration/acp/nac/p2p_document_list_test.go
tests/integration/acp/nac/p2p_peer_connect_test.go
tests/integration/acp/nac/p2p_replicator_create_test.go
tests/integration/acp/nac/p2p_replicator_delete_test.go
tests/integration/acp/nac/relation_admin/index_create_test.go
tests/integration/acp/nac/relation_admin/index_drop_test.go
tests/integration/acp/nac/relation_admin/index_list_test.go
tests/integration/acp/nac/relation_admin/p2p_collection_create_test.go
tests/integration/acp/nac/relation_admin/p2p_collection_delete_test.go
tests/integration/acp/nac/relation_admin/p2p_collection_list_test.go
tests/integration/acp/nac/relation_admin/p2p_document_create_test.go
tests/integration/acp/nac/relation_admin/p2p_document_delete_test.go
tests/integration/acp/nac/relation_admin/p2p_document_list_test.go
tests/integration/acp/nac/relation_admin/p2p_peer_connect_test.go
tests/integration/acp/nac/relation_admin/p2p_replicator_create_test.go
tests/integration/acp/nac/relation_admin/p2p_replicator_delete_test.go
tests/integration/acp/nac/relation_admin/signature_verify_test.go
tests/integration/acp/nac/signature_verify_test.go
tests/integration/acp_dac_setup.go
tests/integration/client.go
tests/integration/client_setup.go
tests/integration/db_setup.go
tests/integration/db_setup_js.go
tests/integration/events.go
tests/integration/explain_result_asserter.go
tests/integration/mutation/update/crdt/pcounter_test.go
tests/integration/net/info/connect_peers_test.go
tests/integration/query/simple/with_max_test.go
tests/integration/query/simple/with_min_test.go
tests/integration/results.go
tests/integration/signature/acp_test.go
tests/integration/signature/branchable_test.go
tests/integration/signature/utils.go
tests/integration/utils.go
tests/state/client_type.go
tests/state/identity.go
tests/state/state.go

🧰 Additional context used

🧬 Code graph analysis (39)

tests/integration/acp/nac/relation_admin/p2p_collection_list_test.go (1)

tests/integration/acp/nac/index_create_test.go (1)

tests/action/assert_request.go (1)

cbindings/errors.go (1)

tests/integration/acp/nac/p2p_peer_connect_test.go (1)

tests/integration/signature/branchable_test.go (1)

tests/integration/utils.go (1)

tests/integration/acp/nac/index_list_test.go (1)

tests/action/results.go (1)

tests/state/identity.go (2)

tests/integration/query/simple/with_min_test.go (1)

tests/integration/acp/nac/relation_admin/p2p_peer_connect_test.go (1)

tests/integration/acp/nac/signature_verify_test.go (1)

tests/integration/acp/nac/relation_admin/signature_verify_test.go (1)

tests/integration/results.go (1)

tests/integration/acp/nac/p2p_collection_list_test.go (1)

tests/integration/acp/nac/p2p_document_create_test.go (1)

tests/integration/acp/nac/relation_admin/p2p_document_create_test.go (1)

tests/integration/acp/nac/p2p_collection_delete_test.go (1)

tests/integration/acp/nac/index_drop_test.go (1)

tests/integration/acp/nac/relation_admin/p2p_document_list_test.go (1)

tests/integration/acp/nac/relation_admin/index_drop_test.go (1)

tests/integration/acp/nac/p2p_document_delete_test.go (1)

tests/integration/signature/utils.go (1)

tests/integration/acp/nac/relation_admin/index_create_test.go (1)

tests/integration/acp/nac/p2p_collection_create_test.go (1)

tests/integration/acp/nac/relation_admin/p2p_document_delete_test.go (1)

tests/integration/acp/nac/relation_admin/p2p_collection_delete_test.go (1)

tests/integration/acp/nac/relation_admin/p2p_replicator_create_test.go (1)

tests/integration/query/simple/with_max_test.go (1)

tests/integration/acp/nac/relation_admin/p2p_replicator_delete_test.go (1)

tests/integration/acp/nac/relation_admin/p2p_collection_create_test.go (1)

tests/integration/acp/nac/p2p_document_list_test.go (1)

tests/integration/mutation/update/crdt/pcounter_test.go (1)

tests/integration/acp/nac/relation_admin/index_list_test.go (1)

tests/integration/client_setup.go (4)

tests/integration/events.go (1)

tests/action/sync_branchable_collection.go (3)

tests/integration/acp/nac/p2p_replicator_create_test.go (1)

🪛 checkmake (0.2.2)

Makefile

[warning] 686-686: Missing required phony target "all"

(minphony)

🪛 Clang (14.0.6)

tests/clients/rustffi/defra.h

[error] 19-19: 'stdlib.h' file not found

(clang-diagnostic-error)

🪛 GitHub Actions: Lint Workflow

tests/clients/rustffi/wrapper.go

[error] 194-194: golangci-lint: Error return value of Close is not checked (errcheck).

🪛 GitHub Actions: Test Coverage Workflow

tests/state/identity.go

[error] 140-140: undefined: cbindings.RegisterIdentityWithRust

🪛 GitHub Check: Lint GoLang job

tests/clients/rustffi/wrapper.go

[failure] 225-225:
use of fmt.Printf forbidden because "Do not commit print statements." (forbidigo)

[failure] 219-219:
use of fmt.Printf forbidden because "Do not commit print statements." (forbidigo)

[failure] 215-215:
use of fmt.Println forbidden because "Do not commit print statements." (forbidigo)

[failure] 211-211:
use of fmt.Printf forbidden because "Do not commit print statements." (forbidigo)

[failure] 204-204:
use of fmt.Println forbidden because "Do not commit print statements." (forbidigo)

[failure] 200-200:
use of fmt.Println forbidden because "Do not commit print statements." (forbidigo)

[failure] 597-597:
Error return value is not checked (errcheck)

[failure] 369-369:
Error return value of (*github.com/sourcenetwork/defradb/tests/clients/rustffi.Node).Close is not checked (errcheck)

[failure] 199-199:
Error return value of (*github.com/sourcenetwork/defradb/tests/clients/rustffi.Subscription).Close is not checked (errcheck)

[failure] 194-194:
Error return value of (*github.com/sourcenetwork/defradb/tests/clients/rustffi.Node).Close is not checked (errcheck)

🪛 GitHub Check: Test coverage JS job

tests/state/identity.go

[failure] 140-140:
undefined: cbindings.RegisterIdentityWithRust

🔇 Additional comments (54)

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

coderabbitai · 2026-02-09T22:46:25Z

client/document.go

+// SetDocID sets the document's ID. This is used by FFI wrappers that need
+// to update the Go-side document ID after Rust applies defaults (e.g. UTC_NOW).
+func (doc *Document) SetDocID(docID DocID) {
+	doc.setDocID(docID)
+}


⚠️ Potential issue | 🟡 Minor

ID() reads without a lock, but SetDocID writes under a lock — pre-existing race widened.

ID() (line 612-615) reads doc.id without any lock, with a comment stating the DocID is assumed immutable. This new public SetDocID (and existing GenerateAndSetDocID) contradicts that assumption. If any goroutine calls SetDocID while another calls ID(), this is a data race on the doc.id struct.

This is a pre-existing issue, but exposing yet another public mutator increases the risk surface. Consider either adding a read-lock to ID() or documenting the concurrency contract clearly.

🤖 Prompt for AI Agents

In `@client/document.go` around lines 973 - 977, ID() currently reads doc.id without synchronization while SetDocID and GenerateAndSetDocID mutate doc.id under a lock, which creates a potential data race; to fix, update the ID() method to acquire the document's read lock (e.g., use doc.mu.RLock()/RUnlock()) around the access to doc.id so reads are synchronized with SetDocID/GenerateAndSetDocID, or alternatively add clear concurrency docs on the public mutators explaining callers must synchronize—reference the ID(), SetDocID, GenerateAndSetDocID methods and the doc.id field when making the change.

coderabbitai · 2026-02-09T22:46:25Z

Makefile

+	@echo "=== Generating defra.h from $(RUST_LIB) ==="
+	@echo "=== Generating defra.h from $(RUST_LIB) ==="


⚠️ Potential issue | 🟡 Minor

Duplicate echo line.

Line 515 is an exact duplicate of Line 514.

Proposed fix

`@echo` "=== Generating defra.h from $(RUST_LIB) ===" - `@echo` "=== Generating defra.h from $(RUST_LIB) ==="

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

@echo "=== Generating defra.h from $(RUST_LIB) ==="

@echo "=== Generating defra.h from $(RUST_LIB) ==="

`@echo` "=== Generating defra.h from $(RUST_LIB) ==="

🤖 Prompt for AI Agents

In `@Makefile` around lines 514 - 515, Remove the duplicated echo invocation; locate the two identical lines '@echo "=== Generating defra.h from $(RUST_LIB) ==="' in the Makefile and delete one of them so only a single echo remains, leaving the generation message printed once.

coderabbitai · 2026-02-09T22:46:25Z

scripts/run-rust-ffi-tests.sh

+TOTAL=$(echo "$RAW_OUTPUT" | grep -c "^=== RUN" || echo 0)
+PASSED=$(echo "$RAW_OUTPUT" | grep -c "^--- PASS" || echo 0)
+FAILED=$(echo "$RAW_OUTPUT" | grep -c "^--- FAIL" || echo 0)


⚠️ Potential issue | 🟡 Minor

grep -c ... || echo 0 produces double output on zero matches.

When grep -c finds no matches, it prints 0 to stdout and exits with status 1, which triggers the || echo 0 fallback. The command substitution captures both outputs, resulting in "0\n0" instead of "0". This would corrupt the arithmetic in the report.

Use || true instead, since grep -c already outputs 0 on no matches:

Proposed fix

-TOTAL=$(echo "$RAW_OUTPUT" | grep -c "^=== RUN" || echo 0) -PASSED=$(echo "$RAW_OUTPUT" | grep -c "^--- PASS" || echo 0) -FAILED=$(echo "$RAW_OUTPUT" | grep -c "^--- FAIL" || echo 0) +TOTAL=$(echo "$RAW_OUTPUT" | grep -c "^=== RUN" || true) +PASSED=$(echo "$RAW_OUTPUT" | grep -c "^--- PASS" || true) +FAILED=$(echo "$RAW_OUTPUT" | grep -c "^--- FAIL" || true)

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

TOTAL=$(echo "$RAW_OUTPUT" | grep -c "^=== RUN" || echo 0)

PASSED=$(echo "$RAW_OUTPUT" | grep -c "^--- PASS" || echo 0)

FAILED=$(echo "$RAW_OUTPUT" | grep -c "^--- FAIL" || echo 0)

TOTAL=$(echo "$RAW_OUTPUT" | grep -c "^=== RUN" || true)

PASSED=$(echo "$RAW_OUTPUT" | grep -c "^--- PASS" || true)

FAILED=$(echo "$RAW_OUTPUT" | grep -c "^--- FAIL" || true)

🤖 Prompt for AI Agents

In `@scripts/run-rust-ffi-tests.sh` around lines 46 - 48, The three count assignments (TOTAL, PASSED, FAILED) use "grep -c ... || echo 0" which can emit duplicate zeros when grep returns no matches; change the fallbacks to "|| true" so the command substitution only captures grep's own "0" output (e.g., update the lines assigning TOTAL, PASSED, and FAILED to use "grep -c ... || true"), keeping the same grep patterns and variable names.

coderabbitai · 2026-02-09T22:46:25Z

tests/action/sync_branchable_collection.go

+
+	// Set up expected merge events for WaitForSync.
+	// Copy document-level heads from connected peers so WaitForSync
+	// knows which MergeComplete events to wait for.
+	if !expectedErrorRaised {
+		for peerID := range nodeState.P2P.Connections {
+			peerNode := a.s.Nodes[peerID]
+			docIDs := a.s.DocIDs[a.CollectionID]
+			for _, docID := range docIDs {
+				docIDStr := docID.String()
+				if head, ok := peerNode.P2P.ActualDAGHeads[docIDStr]; ok {
+					nodeState.P2P.ExpectedDAGHeads[docIDStr] = head.CID
+				}
+			}
+		}
+	}


⚠️ Potential issue | 🟡 Minor

Potential index-out-of-range on a.s.DocIDs[a.CollectionID].

Line 69 accesses a.s.DocIDs[a.CollectionID] without a bounds check. While a.CollectionID is validated against nodeState.Collections (line 49), a.s.DocIDs is a separate slice and may have fewer entries (e.g., if a collection was added but no documents were created for it yet).

Consider guarding:

Proposed fix

if !expectedErrorRaised { + if a.CollectionID >= len(a.s.DocIDs) { + return + } for peerID := range nodeState.P2P.Connections { peerNode := a.s.Nodes[peerID] docIDs := a.s.DocIDs[a.CollectionID]

🤖 Prompt for AI Agents

In `@tests/action/sync_branchable_collection.go` around lines 62 - 77, The code reads a.s.DocIDs[a.CollectionID] without ensuring the index exists, which can panic if DocIDs has fewer entries than the CollectionID; update the loop to safely obtain docIDs by checking that a.CollectionID is within bounds of a.s.DocIDs (e.g., len(a.s.DocIDs) > a.CollectionID) and skip the per-peer copy when missing or when docIDs is nil/empty; keep the rest of the logic (iterating docIDs, using peerNode.P2P.ActualDAGHeads and setting nodeState.P2P.ExpectedDAGHeads) unchanged and reference these symbols (a.s.DocIDs, a.CollectionID, nodeState.Collections, peerNode.P2P.ActualDAGHeads, nodeState.P2P.ExpectedDAGHeads) when applying the guard.

coderabbitai · 2026-02-09T22:46:25Z

tests/clients/rustffi/defra.go

+var (
+	// initOnce ensures Init is only called once
+	initOnce sync.Once
+
+	// ErrNotInitialized is returned when FFI functions are called before Init
+	ErrNotInitialized = errors.New("ffi: not initialized - call Init() first")
+)


⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

rg -n "ErrNotInitialized" --type=go

Repository: sourcenetwork/defradb

Length of output: 284

Remove unused sentinel error ErrNotInitialized.

The error is declared at lines 38–39 but is not referenced anywhere in the codebase. Either add logic to use it for initialization checks or remove it.

🤖 Prompt for AI Agents

In `@tests/clients/rustffi/defra.go` around lines 34 - 40, Remove the unused sentinel error ErrNotInitialized (or use it): either delete the declaration ErrNotInitialized from the var block in defra.go, or update Init and any FFI entry points to return ErrNotInitialized when called before initialization (use the existing initOnce and Init function/logic to gate calls). Ensure references use the exact symbol name ErrNotInitialized and that Init and any public FFI functions check initialization state consistently.

coderabbitai · 2026-02-09T22:46:25Z

tests/clients/rustffi/wrapper.go

+					fmt.Printf("[FFI-MERGE-POLLER] Publishing MergeComplete to Go event bus: doc=%s cid=%s\n",
+						mc.Merge.DocID, mc.Merge.Cid)
+					eb.Publish(event.NewMessage(event.MergeCompleteName, mc))
+					continue
+				}
+				if result.Event.Type == "replicator_completed" {
+					fmt.Println("[FFI-MERGE-POLLER] Publishing ReplicatorCompleted to Go event bus")
+					eb.Publish(event.NewMessage(event.ReplicatorCompletedName, nil))
+					continue
+				}
+				if result.Event.Type == "topic_peer_event" {
+					fmt.Printf("[FFI-MERGE-POLLER] Publishing TopicPeerEvent to Go event bus: peer=%s topic=%s type=%s\n",
+						result.Event.PeerID, result.Event.Topic, result.Event.EventType)
+					eb.Publish(event.NewMessage(event.TopicPeerEventName, event.TopicPeerEvent{
+						PeerID:    result.Event.PeerID,
+						Topic:     result.Event.Topic,
+						EventType: result.Event.EventType,
+					}))
+					continue
+				}
+				if result.Event.Type == "se_artifact_received" {
+					fmt.Printf("[FFI-MERGE-POLLER] Publishing SEArtifactReceived to Go event bus: doc=%s\n",
+						result.Event.DocID)
+					eb.Publish(event.NewMessage(event.SEArtifactReceivedName, event.SEArtifactReceived{
+						DocID: result.Event.DocID,
+					}))
+					continue
+				}
+			}
+			time.Sleep(10 * time.Millisecond)
+		}


⚠️ Potential issue | 🟠 Major

Verbose debug logging throughout the event poller.

Lines 236-238, 242, 247-248, 257-258, and similar fmt.Printf calls throughout this file (also at lines 404, 866-867, 872, 882) will fail the forbidigo lint check. These appear to be development-time debug statements. Consider replacing them all with structured logging at Debug level, or removing them before merge.

🤖 Prompt for AI Agents

In `@tests/clients/rustffi/wrapper.go` around lines 236 - 266, Replace development prints (fmt.Printf / fmt.Println) inside the event poller loop with structured logger calls at Debug level (or remove them); specifically replace the fmt.Printf/fmt.Println usages that log MergeComplete (mc, mc.Merge.DocID, mc.Merge.Cid), replicator_completed, topic_peer_event (result.Event.PeerID, result.Event.Topic, result.Event.EventType), and se_artifact_received (result.Event.DocID) with calls to your logger (not fmt) before calling eb.Publish(event.NewMessage(...)); locate these by looking for fmt.Printf/fmt.Println near usages of eb.Publish, event.MergeCompleteName, event.ReplicatorCompletedName, event.TopicPeerEventName, and event.SEArtifactReceivedName and convert them to debug-level structured logs.

tests/clients/rustffi/wrapper.go

coderabbitai · 2026-02-09T22:46:26Z

tests/clients/rustffi/wrapper.go

+type eventBus struct {
+	closed bool
+	subs   []event.Subscription
+}
+
+func newEventBus() *eventBus {
+	return &eventBus{}
+}
+
+func (e *eventBus) Publish(msg event.Message) {
+	if e.closed {
+		return
+	}
+	// Recover from send-on-closed-channel if Close() races with Publish()
+	defer func() {
+		if r := recover(); r != nil {
+			// Channel was closed between our check and the send; safe to ignore.
+		}
+	}()
+	// Deliver message to all matching subscribers
+	delivered := 0
+	total := len(e.subs)
+	for _, sub := range e.subs {
+		if es, ok := sub.(*eventSubscription); ok {
+			// Check if subscription wants this event
+			if es.wantsEvent(msg.Name) {
+				select {
+				case es.ch <- msg:
+					delivered++
+				default:
+					fmt.Printf("[GO-EVENT-BUS] Channel full for event=%s\n", msg.Name)
+				}
+			}
+		}
+	}
+	if msg.Name == event.MergeCompleteName || msg.Name == event.ReplicatorCompletedName {
+		fmt.Printf("[GO-EVENT-BUS] Publish event=%s total_subs=%d delivered=%d\n", msg.Name, total, delivered)
+	}
+}
+
+func (e *eventBus) Subscribe(events ...event.Name) (event.Subscription, error) {
+	sub := &eventSubscription{
+		ch:     make(chan event.Message, 100),
+		events: events,
+	}
+	e.subs = append(e.subs, sub)
+	fmt.Printf("[GO-EVENT-BUS] Subscribe events=%v total_subs=%d\n", events, len(e.subs))
+	return sub, nil
+}
+
+func (e *eventBus) Unsubscribe(sub event.Subscription) {
+	for i, s := range e.subs {
+		if s == sub {
+			e.subs = append(e.subs[:i], e.subs[i+1:]...)
+			break
+		}
+	}
+}
+
+func (e *eventBus) Close() {
+	if e.closed {
+		return // already closed, prevent double-close panic
+	}
+	e.closed = true
+	for _, sub := range e.subs {
+		if es, ok := sub.(*eventSubscription); ok {
+			close(es.ch)
+		}
+	}
+}


⚠️ Potential issue | 🟠 Major

eventBus has data races on closed flag and subs slice.

The closed field (Line 1837) is read/written without synchronization — Publish reads it (Line 1846), and Close writes it (Line 1899), potentially from different goroutines. Similarly, subs is mutated by Subscribe (Line 1881), Unsubscribe (Line 1889), and iterated by Publish (Line 1858) without any locking.

The recover() in Publish is a band-aid for the send-on-closed-channel race but doesn't protect the subs slice or the closed flag from concurrent access.

Consider adding a sync.RWMutex to protect closed and subs, or use an atomic.Bool for closed and a mutex for subs.

Sketch using sync.RWMutex

type eventBus struct { + mu sync.RWMutex closed bool subs []event.Subscription }

Then acquire mu.RLock() in Publish, mu.Lock() in Subscribe, Unsubscribe, and Close.

🤖 Prompt for AI Agents

In `@tests/clients/rustffi/wrapper.go` around lines 1836 - 1905, The eventBus has data races on the closed flag and subs slice; add a sync.RWMutex field (e.g., mu) to the eventBus struct and use mu.RLock()/mu.RUnlock() in Publish when reading e.closed and iterating e.subs (around wantsEvent/send logic), and use mu.Lock()/mu.Unlock() in Subscribe, Unsubscribe, and Close when appending/removing from e.subs or setting e.closed; keep the recover() as a defensive measure but rely on the mutex to prevent concurrent access races on eventBus, referencing the eventBus type and its methods Publish, Subscribe, Unsubscribe, and Close and the fields closed and subs.

tests/state/identity.go

- Fix 80 golangci-lint issues in wrapper.go (revive, forbidigo, lll, unconvert, staticcheck, errcheck, goconst, gofmt, goimports, forcetypeassert, goheader) - Extract Rust FFI identity registration into build-tagged files to fix js/wasm compilation (cbindings uses CGO, unavailable on js/wasm) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

coderabbitai

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@tests/clients/rustffi/wrapper.go`:
- Around line 2310-2342: convertDateTimeStrings currently converts any
RFC3339-like string to time.Time, which can incorrectly change regular text
fields; update convertDateTimeStrings (or the caller) to only coerce strings to
time.Time when the field is known to be a datetime in the schema: consult the
CollectionVersion.Fields (or other schema metadata available in the test
harness) to check the field type before converting, and otherwise leave plain
strings untouched; if schema info isn't available at call-site, add a parameter
(e.g., fieldPath or a map[string]FieldType) to convertDateTimeStrings to drive
conditional conversion and document the limitation in tests/helpers.
- Around line 2194-2206: In CollectionWrapper.Exists: don’t swallow GraphQL
errors from result.GQL.Errors; instead inspect result.GQL.Errors (from
ExecRequest) and if the first error is a genuine failure (not a "not found"
variant) return false with that error so callers can distinguish failures from
absence; only treat explicit "not found" or no-error cases as (false, nil) —
update the Exists function to return (false, err) using the first
result.GQL.Errors[0] (or wrap it) when appropriate and otherwise continue the
current data parsing logic.
- Around line 2042-2049: The generated GraphQL list for
createDocOpts.EncryptedFields is missing string quotes (producing e.g. [Name,
Age]); update the code that builds params (the block using
createDocOpts.EncryptedFields, local variable fields and strings.Join) to quote
each field name (use strconv.Quote or fmt.Sprintf("%q", f)) before joining so
params contains encryptFields: ["Name","Age"] and properly escapes values.

🧹 Nitpick comments (3)

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1b0b84b and fb07f6a.

📒 Files selected for processing (4)

tests/clients/rustffi/wrapper.go
tests/state/identity.go
tests/state/identity_rust.go
tests/state/identity_rust_js.go

🚧 Files skipped from review as they are similar to previous changes (1)

tests/state/identity.go

🧰 Additional context used

🧬 Code graph analysis (3)

tests/state/identity_rust.go (3)

tests/state/identity_rust_js.go (1)

tests/clients/rustffi/wrapper.go (11)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (48)

GitHub Check: Test macos job
GitHub Check: Test coverage job (c, file, gql)
GitHub Check: Test coverage job (http, memory, gql)
GitHub Check: Test coverage job (cli, file, collection-save)
GitHub Check: Test coverage job (cli, memory, collection-named)
GitHub Check: Test coverage job (c, file, collection-named)
GitHub Check: Test coverage job (c, memory, collection-save)
GitHub Check: Test coverage job (c, file, collection-save)
GitHub Check: Test coverage job (http, memory, collection-named)
GitHub Check: Test coverage job (cli, memory, collection-save)
GitHub Check: Test coverage job (c, memory, collection-named)
GitHub Check: Test coverage job (http, file, collection-named)
GitHub Check: Test coverage job (http, memory, collection-save)
GitHub Check: Test coverage job (cli, memory, gql)
GitHub Check: Test coverage job (cli, file, gql)
GitHub Check: Test coverage job (go, memory, gql)
GitHub Check: Test coverage job (http, file, gql)
GitHub Check: Test coverage job (cli, file, collection-named)
GitHub Check: Test coverage job (c, memory, gql)
GitHub Check: Test coverage job (http, file, collection-save)
GitHub Check: Test coverage job (go, memory, collection-named)
GitHub Check: Test coverage job (go, file, collection-save)
GitHub Check: Test coverage job (go, file, collection-named)
GitHub Check: Test coverage job (go, memory, collection-save)
GitHub Check: Test coverage job (go, file, gql)
GitHub Check: Test Limited Resource job
GitHub Check: Test NPX/JS build job
GitHub Check: Lint GoLang job
GitHub Check: Test coverage document acp job (c, source-hub)
GitHub Check: Test coverage document acp job (go, source-hub)
GitHub Check: Test coverage document acp job (http, source-hub)
GitHub Check: Test coverage document acp job (cli, source-hub)
GitHub Check: Test coverage view job
GitHub Check: Test coverage secondary index job
GitHub Check: Check mocks job
GitHub Check: Test coverage leveldb job
GitHub Check: Test coverage encryption job
GitHub Check: Test coverage lens job (wazero)
GitHub Check: Test coverage telemetry job
GitHub Check: Check data format changes job
GitHub Check: Check cli documentation job
GitHub Check: Check http documentation job
GitHub Check: Check vulnerabilities job
GitHub Check: Validate containerfile job
GitHub Check: Build dependencies job
GitHub Check: Check wizard health job
GitHub Check: Start binary job
GitHub Check: Lint GoLang job

🔇 Additional comments (2)

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

coderabbitai · 2026-02-09T23:10:41Z

tests/clients/rustffi/wrapper.go

+	if createDocOpts.EncryptDoc {
+		params += ", encrypt: true"
+	}
+	if len(createDocOpts.EncryptedFields) > 0 {
+		fields := make([]string, len(createDocOpts.EncryptedFields))
+		copy(fields, createDocOpts.EncryptedFields)
+		params += ", encryptFields: [" + strings.Join(fields, ", ") + "]"
+	}


⚠️ Potential issue | 🟠 Major

encryptFields values are not quoted — produces invalid GraphQL.

When EncryptedFields contains ["Name", "Age"], this generates encryptFields: [Name, Age] (bare identifiers). GraphQL expects encryptFields: ["Name", "Age"] for a [String!] argument. Each field name must be individually quoted.

🐛 Proposed fix

if len(createDocOpts.EncryptedFields) > 0 { - fields := make([]string, len(createDocOpts.EncryptedFields)) - copy(fields, createDocOpts.EncryptedFields) - params += ", encryptFields: [" + strings.Join(fields, ", ") + "]" + quoted := make([]string, len(createDocOpts.EncryptedFields)) + for i, f := range createDocOpts.EncryptedFields { + quoted[i] = `"` + f + `"` + } + params += ", encryptFields: [" + strings.Join(quoted, ", ") + "]" }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

if createDocOpts.EncryptDoc {

params += ", encrypt: true"

}

if len(createDocOpts.EncryptedFields) > 0 {

fields := make([]string, len(createDocOpts.EncryptedFields))

copy(fields, createDocOpts.EncryptedFields)

params += ", encryptFields: [" + strings.Join(fields, ", ") + "]"

}

if createDocOpts.EncryptDoc {

params += ", encrypt: true"

}

if len(createDocOpts.EncryptedFields) > 0 {

quoted := make([]string, len(createDocOpts.EncryptedFields))

for i, f := range createDocOpts.EncryptedFields {

quoted[i] = `"` + f + `"`

}

params += ", encryptFields: [" + strings.Join(quoted, ", ") + "]"

}

🤖 Prompt for AI Agents

In `@tests/clients/rustffi/wrapper.go` around lines 2042 - 2049, The generated GraphQL list for createDocOpts.EncryptedFields is missing string quotes (producing e.g. [Name, Age]); update the code that builds params (the block using createDocOpts.EncryptedFields, local variable fields and strings.Join) to quote each field name (use strconv.Quote or fmt.Sprintf("%q", f)) before joining so params contains encryptFields: ["Name","Age"] and properly escapes values.

coderabbitai · 2026-02-09T23:10:41Z

tests/clients/rustffi/wrapper.go

+func (c *CollectionWrapper) Exists(ctx context.Context, docID client.DocID) (bool, error) {
+	query := fmt.Sprintf(`{ %s(docID: "%s") { _docID } }`, c.version.Name, docID.String())
+	result := c.wrapper.ExecRequest(ctx, query)
+	if len(result.GQL.Errors) > 0 {
+		return false, nil
+	}
+	if data, ok := result.GQL.Data.(map[string]any); ok {
+		if docs, ok := data[c.version.Name].([]any); ok {
+			return len(docs) > 0, nil
+		}
+	}
+	return false, nil
+}


⚠️ Potential issue | 🟡 Minor

Exists swallows all GQL errors as "not found".

Line 2198 returns (false, nil) whenever there are GQL errors, which could mask real failures (e.g. schema errors, connection issues) as a simple "doesn't exist". Consider propagating the first error when it isn't a "not found" variant.

🤖 Prompt for AI Agents

In `@tests/clients/rustffi/wrapper.go` around lines 2194 - 2206, In CollectionWrapper.Exists: don’t swallow GraphQL errors from result.GQL.Errors; instead inspect result.GQL.Errors (from ExecRequest) and if the first error is a genuine failure (not a "not found" variant) return false with that error so callers can distinguish failures from absence; only treat explicit "not found" or no-error cases as (false, nil) — update the Exists function to return (false, err) using the first result.GQL.Errors[0] (or wrap it) when appropriate and otherwise continue the current data parsing logic.

coderabbitai · 2026-02-09T23:10:41Z

tests/clients/rustffi/wrapper.go

+func convertDateTimeStrings(v any) any {
+	switch val := v.(type) {
+	case map[string]any:
+		for k, v := range val {
+			val[k] = convertDateTimeStrings(v)
+		}
+		return val
+	case []any:
+		for i, v := range val {
+			val[i] = convertDateTimeStrings(v)
+		}
+		return val
+	case string:
+		if t, err := time.Parse(time.RFC3339Nano, val); err == nil {
+			return t
+		}
+		if t, err := time.Parse(time.RFC3339, val); err == nil {
+			return t
+		}
+		return val
+	case json.Number:
+		// Try int64 first, then float64
+		if i, err := val.Int64(); err == nil {
+			return i
+		}
+		if f, err := val.Float64(); err == nil {
+			return f
+		}
+		return val
+	default:
+		return val
+	}
+}


⚠️ Potential issue | 🟡 Minor

convertDateTimeStrings will silently coerce any RFC 3339 string field to time.Time.

Regular string fields that happen to contain date-like values (e.g. "2024-01-15T00:00:00Z" stored as a note or description) will be converted to time.Time, diverging from what the Rust FFI actually returned. This could produce hard-to-diagnose false positives/negatives in test assertions.

Consider qualifying the conversion by checking the field's schema type (via CollectionVersion.Fields) rather than doing a blanket recursive walk, or at minimum document this limitation prominently.

🤖 Prompt for AI Agents

In `@tests/clients/rustffi/wrapper.go` around lines 2310 - 2342, convertDateTimeStrings currently converts any RFC3339-like string to time.Time, which can incorrectly change regular text fields; update convertDateTimeStrings (or the caller) to only coerce strings to time.Time when the field is known to be a datetime in the schema: consult the CollectionVersion.Fields (or other schema metadata available in the test harness) to check the field type before converting, and otherwise leave plain strings untouched; if schema info isn't available at call-site, add a parameter (e.g., fieldPath or a map[string]FieldType) to convertDateTimeStrings to drive conditional conversion and document the limitation in tests/helpers.

Move rustffi package behind `rust_ffi` build tag so CI builds without the Rust library don't fail with `-ldefra_ffi` linker errors. Also fix JS/wasm build by syncing db_setup_js.go setupNode signature, and remove cbindings dependency on Rust extern by moving RegisterIdentityWithRust into the rustffi package where it belongs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@tests/clients/rustffi/wrapper.go`:
- Around line 736-740: The early return that delegates to
datastore.CtxTryGetClientTxn -> TxnWrapper.GetCollections bypasses
options.VersionID and options.CollectionSetID filtering; to fix, move the txn
delegation block (the if that checks datastore.CtxTryGetClientTxn and returns
txnW.GetCollections(...)) so it executes after the VersionID/CollectionSetID
special-case handling, or alternatively implement VersionID and CollectionSetID
support inside TxnWrapper.GetCollections; update the GetCollections callsite
and/or TxnWrapper.GetCollections to ensure any provided options.VersionID or
options.CollectionSetID are applied before returning.

🧹 Nitpick comments (4)

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fb07f6a and 5d38333.

📒 Files selected for processing (8)

tests/clients/rustffi/defra.go
tests/clients/rustffi/identity.go
tests/clients/rustffi/wrapper.go
tests/integration/client_setup.go
tests/integration/client_setup_norustffi.go
tests/integration/client_setup_rustffi.go
tests/state/identity_norust.go
tests/state/identity_rust.go

🚧 Files skipped from review as they are similar to previous changes (1)

tests/state/identity_rust.go

🧰 Additional context used

🧬 Code graph analysis (4)

tests/clients/rustffi/identity.go (2)

tests/integration/client_setup_norustffi.go (1)

tests/integration/client_setup_rustffi.go (5)

tests/clients/rustffi/defra.go (3)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (49)

GitHub Check: Check data format changes job
GitHub Check: Start binary job
GitHub Check: Build dependencies job
GitHub Check: Test coverage job (c, file, collection-named)
GitHub Check: Test coverage job (http, file, collection-named)
GitHub Check: Test coverage job (c, file, gql)
GitHub Check: Test coverage job (c, file, collection-save)
GitHub Check: Test coverage job (cli, file, gql)
GitHub Check: Test coverage job (cli, memory, collection-named)
GitHub Check: Test coverage job (http, file, collection-save)
GitHub Check: Test coverage job (c, memory, collection-save)
GitHub Check: Test coverage job (cli, memory, collection-save)
GitHub Check: Test coverage job (go, memory, gql)
GitHub Check: Test coverage job (http, file, gql)
GitHub Check: Test coverage job (c, memory, gql)
GitHub Check: Test coverage job (go, file, gql)
GitHub Check: Test coverage job (cli, file, collection-save)
GitHub Check: Test coverage job (cli, file, collection-named)
GitHub Check: Test coverage job (c, memory, collection-named)
GitHub Check: Test coverage job (go, memory, collection-named)
GitHub Check: Test coverage job (cli, memory, gql)
GitHub Check: Test coverage job (go, memory, collection-save)
GitHub Check: Test coverage job (http, memory, collection-save)
GitHub Check: Test coverage job (http, memory, gql)
GitHub Check: Test coverage job (http, memory, collection-named)
GitHub Check: Test coverage job (go, file, collection-named)
GitHub Check: Test coverage job (go, file, collection-save)
GitHub Check: Test Limited Resource job
GitHub Check: Test coverage document acp job (c, source-hub)
GitHub Check: Test macos job
GitHub Check: Test coverage telemetry job
GitHub Check: Test coverage leveldb job
GitHub Check: Test coverage document acp job (http, source-hub)
GitHub Check: Test coverage document acp job (go, source-hub)
GitHub Check: Test coverage JS job
GitHub Check: Test coverage encryption job
GitHub Check: Test coverage secondary index job
GitHub Check: Test coverage document acp job (cli, source-hub)
GitHub Check: Test coverage lens job (wazero)
GitHub Check: Test coverage view job
GitHub Check: Test NPX/JS build job
GitHub Check: Check mocks job
GitHub Check: Lint GoLang job
GitHub Check: Lint GoLang job
GitHub Check: Validate containerfile job
GitHub Check: Check vulnerabilities job
GitHub Check: Check wizard health job
GitHub Check: Check cli documentation job
GitHub Check: Check http documentation job

🔇 Additional comments (9)

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

coderabbitai · 2026-02-09T23:26:25Z

tests/clients/rustffi/wrapper.go

+	if clientTxn, ok := datastore.CtxTryGetClientTxn(ctx); ok && clientTxn != nil {
+		if txnW, ok := clientTxn.(*TxnWrapper); ok {
+			return txnW.GetCollections(ctx, options)
+		}
+	}


⚠️ Potential issue | 🟠 Major

Early txn delegation bypasses VersionID and CollectionSetID filtering.

When the context carries a TxnWrapper and options.VersionID (or CollectionSetID) is set, the early return at line 738 delegates to TxnWrapper.GetCollections (lines 1697–1741), which does not handle VersionID or CollectionSetID filtering. This silently returns wrong results for those filter combinations.

Move this txn check after the VersionID special-case block (line 757), or extend TxnWrapper.GetCollections to handle all filter options.

🤖 Prompt for AI Agents

In `@tests/clients/rustffi/wrapper.go` around lines 736 - 740, The early return that delegates to datastore.CtxTryGetClientTxn -> TxnWrapper.GetCollections bypasses options.VersionID and options.CollectionSetID filtering; to fix, move the txn delegation block (the if that checks datastore.CtxTryGetClientTxn and returns txnW.GetCollections(...)) so it executes after the VersionID/CollectionSetID special-case handling, or alternatively implement VersionID and CollectionSetID support inside TxnWrapper.GetCollections; update the GetCollections callsite and/or TxnWrapper.GetCollections to ensure any provided options.VersionID or options.CollectionSetID are applied before returning.

- client_setup_rustffi.go: Pass owner identity context when calling GetNACStatus (now NAC-gated), enabling NAC mirroring from Go to Rust - defra.go: Update LensList and P2PPeerInfo to accept and pass identity DID - wrapper.go: Extract identity from context in ListLenses and PeerInfo, propagate NAC authorization errors from PeerInfo Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…dler The Start action handler was missing reconnectPeers(), causing restarted nodes to not re-establish peer connections. This matches the pattern already used in restartNodes(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Add P2PRetryReplicators Go FFI wrapper method - Add p2p_retry_replicators to C header - Add replicatorRetrier interface and call it in reconnectPeers - Add RetryReplicators to Wrapper implementation This ensures that after a node restart and peer reconnection, any documents that were created while the peer was offline get re-pushed to replicators. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…on, and p2p tests Add state.RustFFIClientType to [go js] test groups for tests where Rust FFI behavior matches Go client (bypasses CLI PersistentPreRun). Includes document_delete, collection_patch, collection_truncate, collection_set_active_version, index_create, index_drop, index_list, document_update_with_filter, p2p_peer_info, and relation_admin variants. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…Rust node Add Go bindings for the set_se_encryption_key Rust FFI function. During setupRustFFIClient, extract the SE encryption key from the Go DB via interface type assertion and pass it to the Rust FFI node so it can generate SE artifacts during P2P replication. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…st-compat

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@tests/clients/rustffi/wrapper.go`:
- Around line 389-396: The doc comment describing SetGoNodeCloser is misplaced
above RetryReplicators; move or copy that comment so it immediately precedes the
actual SetGoNodeCloser method (instead of the RetryReplicators function) and
ensure RetryReplicators has an accurate comment describing that it re-pushes
documents to replicator peers; update the comment block around the functions
RetryReplicators and SetGoNodeCloser so each comment matches its corresponding
method name (RetryReplicators and SetGoNodeCloser).

🧹 Nitpick comments (5)

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5d38333 and 14b62ab.

📒 Files selected for processing (22)

tests/clients/rustffi/defra.go
tests/clients/rustffi/defra.h
tests/clients/rustffi/wrapper.go
tests/integration/acp/nac/collection_patch_test.go
tests/integration/acp/nac/collection_set_active_version_test.go
tests/integration/acp/nac/collection_truncate_test.go
tests/integration/acp/nac/document_delete_test.go
tests/integration/acp/nac/document_update_with_filter_test.go
tests/integration/acp/nac/index_create_test.go
tests/integration/acp/nac/index_drop_test.go
tests/integration/acp/nac/index_list_test.go
tests/integration/acp/nac/p2p_peer_info_test.go
tests/integration/acp/nac/relation_admin/collection_patch_test.go
tests/integration/acp/nac/relation_admin/collection_set_active_version_test.go
tests/integration/acp/nac/relation_admin/collection_truncate_test.go
tests/integration/acp/nac/relation_admin/document_delete_test.go
tests/integration/acp/nac/relation_admin/p2p_peer_info_test.go
tests/integration/acp/nac/relation_admin/signature_verify_test.go
tests/integration/acp/nac/signature_verify_test.go
tests/integration/client_setup_rustffi.go
tests/integration/p2p.go
tests/integration/utils.go

🚧 Files skipped from review as they are similar to previous changes (4)

tests/integration/utils.go
tests/integration/acp/nac/index_drop_test.go
tests/integration/client_setup_rustffi.go
tests/integration/acp/nac/signature_verify_test.go

🧰 Additional context used

🧬 Code graph analysis (16)

tests/integration/acp/nac/p2p_peer_info_test.go (1)

tests/integration/acp/nac/collection_patch_test.go (1)

tests/integration/acp/nac/collection_truncate_test.go (1)

tests/integration/acp/nac/collection_set_active_version_test.go (1)

tests/integration/acp/nac/relation_admin/p2p_peer_info_test.go (1)

tests/integration/acp/nac/relation_admin/document_delete_test.go (1)

tests/integration/acp/nac/index_create_test.go (1)

tests/integration/acp/nac/relation_admin/collection_patch_test.go (1)

tests/integration/acp/nac/relation_admin/signature_verify_test.go (1)

tests/integration/acp/nac/relation_admin/collection_set_active_version_test.go (1)

tests/integration/acp/nac/document_delete_test.go (1)

tests/integration/acp/nac/document_update_with_filter_test.go (1)

tests/integration/acp/nac/index_list_test.go (1)

tests/integration/acp/nac/relation_admin/collection_truncate_test.go (1)

tests/integration/p2p.go (3)

tests/clients/rustffi/defra.go (3)

🪛 Clang (14.0.6)

tests/clients/rustffi/defra.h

[error] 19-19: 'stdlib.h' file not found

(clang-diagnostic-error)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (49)

GitHub Check: Test coverage job (cli, memory, collection-named)
GitHub Check: Test coverage job (http, file, collection-save)
GitHub Check: Test coverage job (cli, file, collection-save)
GitHub Check: Test coverage job (http, memory, collection-save)
GitHub Check: Test coverage job (http, memory, gql)
GitHub Check: Test coverage job (c, file, collection-save)
GitHub Check: Test coverage job (c, memory, collection-named)
GitHub Check: Test coverage job (cli, file, gql)
GitHub Check: Test coverage job (go, memory, collection-named)
GitHub Check: Test coverage job (c, memory, collection-save)
GitHub Check: Test coverage job (c, file, collection-named)
GitHub Check: Test coverage job (c, memory, gql)
GitHub Check: Test coverage job (cli, memory, gql)
GitHub Check: Test coverage job (cli, memory, collection-save)
GitHub Check: Test coverage job (c, file, gql)
GitHub Check: Test coverage job (http, file, gql)
GitHub Check: Test coverage job (cli, file, collection-named)
GitHub Check: Test coverage job (go, file, collection-save)
GitHub Check: Test coverage job (http, memory, collection-named)
GitHub Check: Test coverage job (go, file, collection-named)
GitHub Check: Test coverage job (http, file, collection-named)
GitHub Check: Test coverage job (go, memory, gql)
GitHub Check: Test coverage job (go, memory, collection-save)
GitHub Check: Test coverage job (go, file, gql)
GitHub Check: Test coverage JS job
GitHub Check: Test coverage document acp job (cli, source-hub)
GitHub Check: Test coverage telemetry job
GitHub Check: Test coverage leveldb job
GitHub Check: Test coverage document acp job (go, source-hub)
GitHub Check: Test coverage encryption job
GitHub Check: Test coverage lens job (wazero)
GitHub Check: Test coverage document acp job (c, source-hub)
GitHub Check: Test coverage view job
GitHub Check: Test coverage document acp job (http, source-hub)
GitHub Check: Check vulnerabilities job
GitHub Check: Test coverage secondary index job
GitHub Check: Check data format changes job
GitHub Check: Check wizard health job
GitHub Check: Check http documentation job
GitHub Check: Test NPX/JS build job
GitHub Check: Check cli documentation job
GitHub Check: Start binary job
GitHub Check: Test Limited Resource job
GitHub Check: Test macos job
GitHub Check: Build dependencies job
GitHub Check: Check mocks job
GitHub Check: Lint GoLang job
GitHub Check: Validate containerfile job
GitHub Check: Lint GoLang job

🔇 Additional comments (22)

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

coderabbitai · 2026-02-10T18:22:14Z

tests/clients/rustffi/wrapper.go

+// SetGoNodeCloser sets a callback to close the Go node during wrapper Close().
+// This ensures the Go node's badger lock is released on restart.
+// RetryReplicators re-pushes all existing documents to connected replicator peers.
+func (w *Wrapper) RetryReplicators() error {
+	if w.node == nil {
+		return nil
+	}
+	return w.node.P2PRetryReplicators()


⚠️ Potential issue | 🟡 Minor

Stale/misplaced doc comment on SetGoNodeCloser.

The comment at line 389-391 describes SetGoNodeCloser, but the function immediately following is RetryReplicators. The SetGoNodeCloser function is at line 407. This is a copy-paste artifact.

Proposed fix

-// SetGoNodeCloser sets a callback to close the Go node during wrapper Close(). -// This ensures the Go node's badger lock is released on restart. // RetryReplicators re-pushes all existing documents to connected replicator peers. func (w *Wrapper) RetryReplicators() error {

🤖 Prompt for AI Agents

In `@tests/clients/rustffi/wrapper.go` around lines 389 - 396, The doc comment describing SetGoNodeCloser is misplaced above RetryReplicators; move or copy that comment so it immediately precedes the actual SetGoNodeCloser method (instead of the RetryReplicators function) and ensure RetryReplicators has an accurate comment describing that it re-pushes documents to replicator peers; update the comment block around the functions RetryReplicators and SetGoNodeCloser so each comment matches its corresponding method name (RetryReplicators and SetGoNodeCloser).

- Remove all fmt.Println/Printf debug statements (forbidigo lint violations) - Fix unchecked Close() errors in merge poller goroutine - Add sync.RWMutex to eventBus to prevent data races on closed/subs - Remove redundant early txn delegation that bypassed VersionID/CollectionSetID filtering - Quote encryptFields values in GraphQL mutations - Add RLock to Document.ID() for thread safety with SetDocID - Remove duplicate echo line in Makefile - Fix grep -c double output in run-rust-ffi-tests.sh - Add bounds check for DocIDs[CollectionID] in sync_branchable_collection - Remove unused ErrNotInitialized sentinel error - Handle json.Marshal error in P2PDeleteReplicator - Propagate GQL errors in CollectionWrapper.Exists instead of swallowing - Document convertDateTimeStrings RFC 3339 coercion limitation - Fix misplaced doc comment on RetryReplicators/SetGoNodeCloser Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…gression - Remove 6 fmt.Printf debug statements from tests/integration/events.go that fail forbidigo lint - Fix line-length violation in SE sync timeout message - Remove fmt import (no longer needed) - Revert ExpectedDAGHeads setup in sync_branchable_collection.go that caused BranchableCollectionSync tests to timeout waiting for merge events that don't arrive in the Go test suite Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The ExpectedDAGHeads setup after branchable collection sync is only needed for the Rust FFI client where merge events arrive asynchronously via the poller goroutine. The Go client processes merges synchronously, so WaitForSync doesn't need to wait for merge events in that flow. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Call reconnectPeers() at the end of startNodes() so Rust P2P hosts re-establish connections after individual node restarts, not just full restarts via restartNodes(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

jackzampolin and others added 19 commits January 26, 2026 15:15

fix: Update scripts to use DEFRA_RS_PATH env var

cf9f1f4

Scripts now require DEFRA_RS_PATH to be set and will validate it exists before running tests. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

fix: Don't skip network/backup tests for Rust FFI

d0d5c8b

Run all tests so we can track complete progress. Let them fail if they fail rather than skipping. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

fix: Correct test counting and update gitignore

6590372

- Fix double-counting bug in run-rust-ffi-tests.sh (count only === RUN lines) - Add reports/ and Rust library files to .gitignore Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Revert "fix(ffi): Handle move/copy operations in patch collection rou…

846a443

…ting" This reverts commit c439830.

jackzampolin force-pushed the jack/ffi-rust-compat branch from 88acb06 to 2ae5333 Compare January 29, 2026 17:34

jackzampolin and others added 7 commits January 29, 2026 09:40

feat: Show run metadata in FFI test report

ec801fc

Display the Rust branch, commit, worktree name, and timestamp at the top of test:ffi-report output so it's clear which build produced the results. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

chore: Update defra.h from latest Rust FFI build

9a8163c

Regenerated from defradb.rs main@83dd36f (PR #248 ffi-complete merge). Removes NewTxnResult struct, updates comment formatting. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

jackzampolin and others added 11 commits February 5, 2026 10:42

Merge ffi/view: Add materialized view support

0946703

Brings in FFI binding updates and test fixes for materialized view caching. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

feat(rustffi): Enable NAC signature verify tests for RustFFI client

52aa042

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

feat(rustffi): Update FFI header for subscription support

79ca6be

Update auto-generated defra.h to reflect new subscription FFI functions and updated doc comments for poll/close_graphql_subscription. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

docs(rustffi): Update subscription FFI header comments

00800eb

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

jackzampolin marked this pull request as ready for review February 9, 2026 22:39

coderabbitai bot reviewed Feb 9, 2026

View reviewed changes

jackzampolin and others added 6 commits February 10, 2026 07:18

Merge remote-tracking branch 'origin/ffi/encryption' into jack/ffi-ru…

14b62ab

…st-compat

coderabbitai bot reviewed Feb 10, 2026

View reviewed changes

jackzampolin and others added 5 commits February 10, 2026 13:04

Merge branch 'ffi/net' into jack/ffi-rust-compat

bdc29f3

		@echo "=== Generating defra.h from $(RUST_LIB) ==="
		@echo "=== Generating defra.h from $(RUST_LIB) ==="

	@echo "=== Generating defra.h from $(RUST_LIB) ==="
	@echo "=== Generating defra.h from $(RUST_LIB) ==="
	`@echo` "=== Generating defra.h from $(RUST_LIB) ==="

Conversation

jackzampolin commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What This PR Does

Key Components

New Files

Modified Files (non-trivial)

Merged from develop

Architecture

How to Run

Remaining Failures & Skips

Failures (4 tests)

Skips (88 tests)

Test Plan

Uh oh!

coderabbitai bot commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related issues

Possibly related PRs

Suggested labels

Suggested reviewers

Uh oh!

codecov bot commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

jackzampolin commented Jan 26, 2026 •

edited

Loading

coderabbitai bot commented Jan 26, 2026 •

edited

Loading

codecov bot commented Jan 26, 2026 •

edited

Loading