feat(react-native): Expo module for on-device Moss (#432) - #473
Open
samanyugoyal2010 wants to merge 10 commits into
Open
feat(react-native): Expo module for on-device Moss (#432)#473samanyugoyal2010 wants to merge 10 commits into
samanyugoyal2010 wants to merge 10 commits into
Conversation
Introduce @moss-dev/moss-react-native under sdks/react-native with an Expo Modules SharedObject wrapping Moss.xcframework on iOS, an Android stub pointing at usemoss#411, a config plugin, and a usage example. Co-authored-by: Cursor <cursoragent@cursor.com>
Raise the iOS deployment target to 16.4, wire MossC module/search paths and system frameworks, and exclude x86_64 simulator so prebuild + link succeed on Apple Silicon. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
@cubic.dev |
Contributor
@samanyugoyal2010 I can't start this review because you've reached your trial's review limit. Trial plans have lower review limits than paid plans. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
Codex reviewNo issues found. |
Three fixes from review on usemoss#473: - MossClientSharedObject: borrow() returned the raw handle after dropping the lock, so a JS `client.query(...); client.close();` could free the handle while an async native call was still using it. Every operation is an AsyncFunction running off the JS thread while close() is a synchronous Function on it, so the race was reachable. Replace borrow() with withHandle(), which refcounts in-flight calls under an NSCondition; close() now marks the client closed, drains outstanding calls, and only then calls moss_client_free. Concurrent operations still run in parallel — only teardown serializes against them. - app.plugin.js: the plugin only set ios.deploymentTarget when it was missing, so an app pinning a lower value (e.g. 15.1) kept a target that CocoaPods rejects against the podspec's 16.4. Parse the existing value and raise it whenever it is lower, unset, or unparseable. Comparison is numeric per component, so 16.10 is correctly treated as newer than 16.4. - parseIndexInfo: nullable C strings were boxed with `as Any`, putting Optional.none into the dictionary handed to the bridge. Build the dictionary conditionally instead, matching the existing style in parseSearchResult; the omitted keys are already optional in IndexInfo and ModelRef. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two more fixes from review on usemoss#473: - Numeric options were converted straight from the bridged Double, so `UInt64((options["pollingIntervalSeconds"] as? Double) ?? 600)` trapped on NaN, ±Infinity, negatives and oversized JS numbers — an uncatchable Swift runtime trap that kills the app process before any error can be bridged back. Reproduced locally: "Fatal error: Double value cannot be converted to UInt64 because it is either infinite or NaN". topK and alpha had the same shape. Added integerOption / unitIntervalOption, which check isFinite and explicit bounds and throw a MossError for invalid values before converting. Bounds stay well inside 2^53 so the Double comparisons are exact. Accepts Int/NSNumber as well as Double, since the bridge does not guarantee which it hands over. - query() hard-coded `embedding: nil`, so an index created from custom document embeddings — which the JS wrapper auto-selects by setting modelId 'custom' whenever a doc carries an embedding — could be built but never queried with a query vector. Added `embedding?: number[]` to QueryOptions, marshalled into MossQueryOptions via withUnsafeBufferPointer (same shape as MossSession.query in the Swift SDK) so the buffer stays alive across the call. Non-finite components are rejected up front. Documented the pairing in the README and on MossClient.query. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…osure Two more findings from review on usemoss#473: - QueryOptions only accepted `filterJson`, so a caller migrating from `@moss-dev/moss` — which takes `filter?: Record<string, unknown>` — would have their filter silently dropped and get an unfiltered query returning documents outside the intended scope. Added `filter` with the same shape as the Node SDK, serialized to the engine's JSON form. Passing both `filter` and `filterJson` is an error rather than a silent precedence rule, and an unserializable filter raises a MossError instead of falling back to null. - The quick start put the project key in `EXPO_PUBLIC_MOSS_PROJECT_KEY`. Expo inlines `EXPO_PUBLIC_*` into the shipped JS bundle, so that key is readable by anyone with the app, and MossClient also exposes createIndex / addDocs / deleteIndex — a leaked key is not just read access. Documented this in a Credentials section, marked the quick start and the example as development-only, and pointed at the Authenticator path (moss_client_new_with_authenticator) as the intended fix. Note: the Authenticator/session bridge itself is NOT implemented here — Session / Authenticator APIs are deferred for this first release, so an embedded project key must be treated as public until it lands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@HarshaNalluru you can review and merge |
…narrowing Two findings from review on usemoss#473, plus a same-class sweep: - MossReactNative.podspec relied on `prepare_command` to fetch Moss.xcframework. CocoaPods only runs that for pods it downloads into Pods/ — Expo/RN autolinking installs this one from node_modules as a development pod (`:path`), which skips it. Combined with .npmignore excluding ios/Frameworks/, `vendored_frameworks` pointed at a directory that never existed. The download now runs while the podspec is evaluated, which happens before CocoaPods resolves vendored_frameworks and on every install path. Verified end to end: evaluating the podspec fetches and checksum-verifies the binary, and a second run is a no-op. - embeddingOption checked `isFinite` on the Double before narrowing, but a finite Double above Float.greatestFiniteMagnitude (~3.4e38) rounds to Float.infinity, so oversized query embeddings still reached the engine as non-finite. Now narrows first and checks the Float. 1e300 is rejected, 3.4e38 still accepted. Same-class sweep (not flagged, same defect shape as findings already raised): createIndex/addDocs passed docs straight to JSON.stringify, which silently rewrites NaN/±Infinity to null — a corrupted embedding reaching the engine rather than an error. Added serializeDocs, which rejects non-finite and out-of-Float-range embedding values and reports an unserializable doc set as a MossError instead of a raw TypeError. Bounds match the native check exactly. MossClientSharedObject.swift now type-checks against the real MossC header from the xcframework (ExpoModulesCore/UIKit stubbed), confirming the MossQueryOptions field order and the embedding marshalling. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…Json Two findings from review on usemoss#473: - close() waited for in-flight calls before freeing the handle. It is exposed as a synchronous JS `Function`, so that wait ran on the JS thread and could freeze the UI for as long as a slow loadIndex / query / mutation took — component cleanup and error paths being the obvious cases. Kept the use-after-free protection but handed ownership of the free to whoever is last out: close() marks the client closed and frees only if nothing is in flight, otherwise the final release() does it. acquire() still refuses once closed, so no new work can start either way. NSCondition is now an NSLock since nothing waits. Verified with a 400-trial concurrency simulation of the exact logic (8 workers racing close()): 400 frees for 400 trials, so no double-free and no leak, zero use-after-free, and close() returns in under 2µs. - resolveFilterJson returned `options?.filterJson ?? null` unchecked. A plain-JS caller passing a non-string would have it fail the Swift side's `as? String` cast and run the query *unfiltered* — for a tenant- or user-scoping filter that silently returns documents the caller never intended to expose. Now throws when filterJson is present but not a string; undefined/null still mean "no filter". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The `files` allow-list included the whole `ios` directory, and the root
.npmignore did not exclude `ios/Frameworks/` from it. Confirmed by packing
with the framework present, as it is after any local `pod install`:
before: 35 files, 18.4 MB packed / 73.7 MB unpacked
(two 36.8 MB libmoss.a slices shipped)
after: 28 files, 20.2 kB packed / 63.7 kB unpacked
(zero Frameworks entries)
Beyond the size, a packed copy is the more serious problem: the download
script short-circuits when Frameworks/ already exists, so a consumer would
get whatever binary the publisher happened to have on disk, with the
checksum verification skipped entirely.
`files` now enumerates the iOS sources, podspec and scripts explicitly
rather than the whole directory. Added ios/.npmignore as a second line of
defence if that list is ever widened back. Verified the tarball still
carries both Swift sources, the podspec and the download script.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@HarshaNalluru this can be merged |
Addresses the credentials finding on usemoss#473: the package previously offered only a long-lived project key while also exposing createIndex / addDocs / deleteIndex, so any production app had to ship a mutating secret in its JS bundle. `new MossClient({ projectId, getAuthToken })` now constructs a client via moss_client_new_with_authenticator_and_device_id, so no long-lived secret is embedded. The existing `new MossClient(projectId, projectKey)` form is unchanged for development builds. The native contract is already request-id based and asynchronous, which maps onto Expo's event pattern without blocking anything: native needs a token -> MossAuthNotifyFn trampoline (arbitrary Rust thread, must not block) -> sendEvent("onMossAuthRequest", {clientId, requestId}) -> JS awaits the caller's getAuthToken() -> moss_resolve_auth_request / moss_reject_auth_request Details worth noting: - The AuthBridgeBox handed over as `user_data` is retained manually and released only *after* moss_client_free, since the native side can invoke the callback until the client is gone. Released on every constructor error path too. - JS answers on every path. An unanswered request would leave the native call that triggered it waiting forever, so a missing, throwing, or non-string-returning provider still produces a rejection — verified across 9 scenarios including reject() itself failing. - requestId is range-checked before narrowing to UInt32 rather than trapping, consistent with the other bridged numbers. Not verified here: MossModule.swift cannot be compiled without the real ExpoModulesCore, and the end-to-end token round trip needs a device. MossClientSharedObject.swift does type-check against the real MossC header from the xcframework. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@moss-dev/moss-react-nativeundersdks/react-native/— an Expo Modules package that exposes aMossClientSharedObject for on-device semantic search.Moss.xcframework(same binary as the Swift SDK,v0.6.2), downloaded + checksum-verified atpod install.examples/react-native/usage sketch. Updates ROADMAP / AGENTS / root README.Closes #432
Notes
expo prebuild(not Expo Go).createIndex,loadIndex,query,addDocs, index CRUD). Session / Authenticator APIs from the Swift SDK are deferred.Test plan
cd sdks/react-native && npm install && npm run buildplugins: ["@moss-dev/moss-react-native"], runnpx expo prebuild+pod install, confirmMoss.xcframeworkdownloadsMossClientthrows with the SDK: Kotlin / Android bindings #411 messageMade with Cursor