feat(rn-sdk): keychain-backed default DocumentsStore - #2263
Conversation
DocumentsHandler's default store was in-memory, so hosts that don't inject
a documents store (example app, partner apps) lost every captured document
on restart. The default is now keychain-backed via react-native-keychain
(already an optional peer dep), with a one-time-warn in-memory fallback
when the module is absent (pure-JS tests, shells without keychain).
Services are self_docs_catalog / self_docs_doc_<id> — deliberately outside
KeychainHandler's collidable self_sdk_${key} namespace and disjoint from
the Self app's own documentCatalog / document-<hash> services; the doc_
infix keeps a document id from colliding with the catalog service. Service
names are pinned by tests (renames lose user data).
First half of the documents-domain swap; spec:
specs/projects/sdk/workstreams/webview-in-app/plans/WIA-documents-bridge-domain.md
Validation: rn-sdk 207 tests + types green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (3)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d51094bbf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export type { NavigationCallbacks } from './NavigationHandler'; | ||
| export { DocumentsHandler } from './DocumentsHandler'; | ||
| export type { DocumentsStore } from './DocumentsHandler'; | ||
| export { createKeychainDocumentsStore } from './KeychainDocumentsStore'; |
There was a problem hiding this comment.
Consumers of the published package cannot import this new factory: package.json exposes only the . entry point, while src/index.ts explicitly re-exports selected names from this internal barrel and omits createKeychainDocumentsStore. As a result, the export added here is unreachable through @selfxyz/rn-sdk; add it to the root export list if this factory is intended to be part of the announced API.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,69 @@ | |||
| # WIA — Documents via the `documents` bridge domain | |||
There was a problem hiding this comment.
This new PR plan is named WIA-documents-bridge-domain.md, but repository instructions require plan filenames to use plans/<BACKLOG-ID>-<slug>.md; WIA is only the workstream prefix rather than an individual backlog ID such as the neighboring WIA-09 or WIA-17. Assign the plan a concrete backlog ID and update references to the filename.
AGENTS.md reference: AGENTS.md:L81-L87
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| async function write(service: string, username: string, value: unknown): Promise<void> { | ||
| await module!.setGenericPassword(username, JSON.stringify(value ?? null), { service }); |
There was a problem hiding this comment.
When setGenericPassword resolves to false—a result explicitly permitted by KeychainModule—this helper still resolves successfully, so DocumentsHandler acknowledges the save to the WebView even though the document or catalog was not persisted. The UI can then proceed as if capture succeeded and the data disappears after relaunch; check the boolean and throw on false so persistence failures fail closed.
AGENTS.md reference: AGENTS.md:L34-L36
Useful? React with 👍 / 👎.
1 participant
Stacked on #2262 (which stacks on #2245). PR 1 of 2 for the documents-domain swap — spec:
specs/projects/sdk/workstreams/webview-in-app/plans/WIA-documents-bridge-domain.md.Problem
DocumentsHandler's default store is in-memory, so any host that doesn't inject adocumentsstore — the example app and any partner app using@selfxyz/rn-sdkout of the box — silently loses every captured document on app restart. This becomes load-bearing when webview-app switches its document I/O to thedocumentsbridge domain (PR 2).Change
createKeychainDocumentsStore()(packages/rn-sdk/src/handlers/KeychainDocumentsStore.ts):DocumentsStorebacked by react-native-keychain (already an optional peer dep), values as JSON strings in generic-password entries, corrupted/missing entries load asnull, idempotent delete. Returnsnullwhen the module is absent.DocumentsHandlerdefault chain: injected store → keychain store → one-time-warn in-memory fallback (pure-JS tests, shells without keychain). Hosts passingdocuments(the WIA host) are unaffected.self_docs_catalog/self_docs_doc_<id>: outsideKeychainHandler's collidableself_sdk_${key}namespace, disjoint from the Self app's owndocumentCatalog/document-<hash>services, and thedoc_infix keeps a document id (even the literalcatalog) from colliding with the catalog service. Names are pinned by tests — renaming loses user data.capabilities.secureStoragealready signals keychain availability (same module).Risks
Validation
packages/rn-sdk: 207/207 tests (7 new: service-name pins, round-trips, corrupt-JSON, collision guard, module-absent fallback),pnpm typesgreen.🤖 Generated with Claude Code