feat: add sdk-alpha package with MRZ helper #851
Conversation
WalkthroughThis change introduces the new Changes
Sequence Diagram(s)sequenceDiagram
participant AppScreen as App Screen/Component
participant SDK as @selfxyz/sdk-alpha
participant Adapters as Adapters (Scanner, Network, etc.)
AppScreen->>SDK: createSelfClient({ config, adapters })
AppScreen->>SDK: scanDocument(opts)
SDK->>Adapters: scanner.scan(opts)
Adapters-->>SDK: ScanResult (MRZ/QR/NFC)
SDK-->>AppScreen: ScanResult
AppScreen->>SDK: validateDocument({ scan })
SDK-->>AppScreen: ValidationResult
AppScreen->>SDK: checkRegistration({ scan })
SDK->>Adapters: network.checkRegistration()
Adapters-->>SDK: RegistrationStatus
SDK-->>AppScreen: RegistrationStatus
AppScreen->>SDK: generateProof(req, opts)
SDK->>Adapters: network/crypto adapters
Adapters-->>SDK: ProofHandle
SDK-->>AppScreen: ProofHandle
AppScreen->>SDK: on(event, callback)
SDK-->>AppScreen: Unsubscribe function
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Suggested reviewers
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (8)
packages/sdk-alpha/src/adapters/web/shims.ts (1)
6-7: Consider making stub data more obviously fake.The stubbed QR data
"self://stub-qr"might be consumed as real data in development. Consider using a more obviously fake format like"stub://qr-scan-result"to prevent confusion.- return { mode: "qr", data: "self://stub-qr" }; + return { mode: "qr", data: "stub://qr-scan-result" };packages/sdk-alpha/src/index.ts (1)
7-8: Clarify temporary nature of MRZ exports.The comment suggests these exports are experimental ("to prove structure works"). Consider updating the comment to reflect whether this is the intended final API or if further migration is planned.
-// expose initial processing helper to prove structure works +// MRZ processing utilities migrated from app-local helpers export { extractMRZInfo, formatDateToYYMMDD } from "./processing/mrz.js";packages/sdk-alpha/README.md (1)
31-31: Clarify WebCrypto fallback strategy.The crypto adapter strategy mentions "WebCrypto-first via adapter;
@noble/*fallback; no Node crypto" but doesn't specify when or how the fallback occurs. Consider adding more detail about the fallback conditions.packages/sdk-alpha/src/errors.ts (1)
1-7: Enhance type safety and add documentation for public error APIThe error utilities look good but could benefit from better TypeScript typing and JSDoc documentation since this is a public SDK API.
+/** + * Categories for SDK errors to enable proper error handling + */ export type SdkErrorCategory = "scanner" | "network" | "protocol" | "proof" | "crypto" | "validation" | "config"; +/** + * Extended Error interface with SDK-specific properties + */ +export interface SdkError extends Error { + code: string; + category: SdkErrorCategory; + retryable: boolean; +} + +/** + * Creates a standardized SDK error with categorization and retry semantics + * @param message - Human-readable error message + * @param code - Unique error code for programmatic handling + * @param category - Error category for grouping and handling + * @param retryable - Whether the operation can be safely retried + */ -export function sdkError(message: string, code: string, category: SdkErrorCategory, retryable = false) { +export function sdkError(message: string, code: string, category: SdkErrorCategory, retryable = false): SdkError { - return Object.assign(new Error(message), { code, category, retryable }); + return Object.assign(new Error(message), { code, category, retryable }) as SdkError; } +/** + * Creates a standardized "not implemented" error for missing adapters + * @param name - Name of the missing adapter + */ -export function notImplemented(name: string) { +export function notImplemented(name: string): SdkError { return sdkError(`${name} adapter not provided`, "SELF_ERR_ADAPTER_MISSING", "config", false); }packages/sdk-alpha/src/client.ts (2)
27-29: Stub implementation should indicate development statusThe hardcoded failure return makes it unclear whether this is intentional behavior or a placeholder. Consider using a more explicit approach.
async function validateDocument(_input: ValidationInput): Promise<ValidationResult> { - return { ok: false, reason: "SELF_ERR_VALIDATION_STUB" }; + // TODO: Implement document validation logic + throw sdkError("Document validation not yet implemented", "SELF_ERR_NOT_IMPLEMENTED", "validation", false); }This makes it clear to developers that the feature isn't ready rather than appearing as a validation failure.
31-34: Stub implementation should indicate development statusSimilar to the validation method, this stub should be more explicit about its unimplemented state.
async function checkRegistration(_input: RegistrationInput): Promise<RegistrationStatus> { if (!adapters.network) throw notImplemented("network"); - return { registered: false, reason: "SELF_REG_STATUS_STUB" }; + // TODO: Implement registration status check + throw sdkError("Registration check not yet implemented", "SELF_ERR_NOT_IMPLEMENTED", "network", true); }packages/sdk-alpha/src/types/public.ts (2)
18-18: Improve type safety for proof request payloadUsing
unknownfor the payload reduces type safety. Consider using a more specific type or generic constraint.-export interface ProofRequest { type: "register" | "dsc" | "disclose"; payload: unknown; } +export interface ProofRequest<T = Record<string, unknown>> { + type: "register" | "dsc" | "disclose"; + payload: T; +}This allows consumers to specify the payload type while maintaining type safety.
45-65: Add JSDoc documentation for public adapter interfacesSince these are public SDK interfaces that developers will implement, they need comprehensive documentation explaining the expected behavior and requirements.
+/** + * Storage adapter interface for persistent key-value storage + */ export interface StorageAdapter { get(key: string): Promise<string | null>; set(key: string, value: string): Promise<void>; remove(key: string): Promise<void>; } +/** + * Scanner adapter interface for document scanning capabilities + */ export interface ScannerAdapter { scan(opts: ScanOpts & { signal?: AbortSignal }): Promise<ScanResult>; } +/** + * Cryptographic adapter interface for hashing and signing operations + */ export interface CryptoAdapter { hash(input: Uint8Array, algo?: "sha256"): Promise<Uint8Array>; sign(data: Uint8Array, keyRef: string): Promise<Uint8Array>; } +/** + * HTTP adapter interface wrapping fetch-like functionality + */ export interface HttpAdapter { fetch(input: RequestInfo, init?: RequestInit): Promise<Response>; } +/** + * WebSocket connection interface with event handlers + */ export interface WsConn { send: (data: string | ArrayBufferView | ArrayBuffer) => void; close: () => void; onMessage: (cb: (data: any) => void) => void; onError: (cb: (e: any) => void) => void; onClose: (cb: () => void) => void; } +/** + * WebSocket adapter interface for real-time communication + */ export interface WsAdapter { connect(url: string, opts?: { signal?: AbortSignal; headers?: Record<string, string> }): WsConn; } +/** + * Network adapter combining HTTP and WebSocket capabilities + */ export interface NetworkAdapter { http: HttpAdapter; ws: WsAdapter; } +/** + * Clock adapter interface for time-related operations + */ export interface ClockAdapter { now(): number; sleep(ms: number, signal?: AbortSignal): Promise<void>; } +/** + * Log levels supported by the SDK + */ export type LogLevel = "debug" | "info" | "warn" | "error"; +/** + * Logger adapter interface for structured logging + */ export interface LoggerAdapter { log(level: LogLevel, message: string, fields?: Record<string, unknown>): void; } +/** + * Complete set of adapters required by the SDK + */ export interface Adapters { storage: StorageAdapter; scanner: ScannerAdapter; crypto: CryptoAdapter; network: NetworkAdapter; clock: ClockAdapter; logger: LoggerAdapter; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (20)
app/package.json(1 hunks)app/src/components/native/PassportCamera.tsx(1 hunks)app/src/components/native/PassportCamera.web.tsx(1 hunks)app/src/screens/passport/PassportCameraScreen.tsx(2 hunks)app/src/utils/utils.ts(0 hunks)app/tsconfig.json(1 hunks)package.json(1 hunks)packages/sdk-alpha/README.md(1 hunks)packages/sdk-alpha/package.json(1 hunks)packages/sdk-alpha/scripts/validate-exports.mjs(1 hunks)packages/sdk-alpha/scripts/verify-conditions.mjs(1 hunks)packages/sdk-alpha/src/adapters/index.ts(1 hunks)packages/sdk-alpha/src/adapters/web/shims.ts(1 hunks)packages/sdk-alpha/src/client.ts(1 hunks)packages/sdk-alpha/src/config/defaults.ts(1 hunks)packages/sdk-alpha/src/errors.ts(1 hunks)packages/sdk-alpha/src/index.ts(1 hunks)packages/sdk-alpha/src/processing/mrz.ts(1 hunks)packages/sdk-alpha/src/types/public.ts(1 hunks)packages/sdk-alpha/tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- app/src/utils/utils.ts
🧰 Additional context used
📓 Path-based instructions (1)
app/src/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit Configuration File
app/src/**/*.{ts,tsx,js,jsx}: Review React Native TypeScript code for:
- Component architecture and reusability
- State management patterns
- Performance optimizations
- TypeScript type safety
- React hooks usage and dependencies
- Navigation patterns
Files:
app/src/screens/passport/PassportCameraScreen.tsxapp/src/components/native/PassportCamera.tsxapp/src/components/native/PassportCamera.web.tsx
🧠 Learnings (8)
📓 Common learnings
Learnt from: transphorm
PR: selfxyz/self#795
File: app/src/screens/prove/ProveScreen.tsx:5-5
Timestamp: 2025-07-28T19:18:48.270Z
Learning: The import path 'selfxyz/common/utils/scope' is valid when the common package has a "./utils/*" export mapping in package.json, even if ESLint shows resolution errors before the package is built. The package.json exports field properly maps "./utils/*" to the compiled files in dist/.
📚 Learning: 2025-07-12T22:00:02.041Z
Learnt from: transphorm
PR: selfxyz/self#636
File: app/src/utils/cameraPermission.ts:19-21
Timestamp: 2025-07-12T22:00:02.041Z
Learning: The Android permission handling in app/src/utils/cameraPermission.ts is robust and properly implemented using PermissionsAndroid.request() and PermissionsAndroid.check(). Previous issues with Android permission assumptions have been resolved in the current implementation.
Applied to files:
app/src/screens/passport/PassportCameraScreen.tsxapp/src/components/native/PassportCamera.tsxapp/src/components/native/PassportCamera.web.tsx
📚 Learning: 2025-07-16T02:20:44.173Z
Learnt from: transphorm
PR: selfxyz/self#636
File: app/src/components/native/QRCodeScanner.tsx:135-142
Timestamp: 2025-07-16T02:20:44.173Z
Learning: In app/src/components/native/QRCodeScanner.tsx, the Android camera dimension multipliers (screenWidth * 3 and screenHeight * 2) are intentionally set to these values and should not be changed. These multipliers are correct and any visual issues with black areas in the camera preview are caused by other factors, not the dimension calculations.
Applied to files:
app/src/screens/passport/PassportCameraScreen.tsxapp/src/components/native/PassportCamera.tsxapp/src/components/native/PassportCamera.web.tsx
📚 Learning: 2025-07-28T19:18:48.270Z
Learnt from: transphorm
PR: selfxyz/self#795
File: app/src/screens/prove/ProveScreen.tsx:5-5
Timestamp: 2025-07-28T19:18:48.270Z
Learning: The import path 'selfxyz/common/utils/scope' is valid when the common package has a "./utils/*" export mapping in package.json, even if ESLint shows resolution errors before the package is built. The package.json exports field properly maps "./utils/*" to the compiled files in dist/.
Applied to files:
app/package.jsonpackage.jsonapp/src/components/native/PassportCamera.web.tsxapp/tsconfig.jsonpackages/sdk-alpha/scripts/verify-conditions.mjspackages/sdk-alpha/tsconfig.jsonpackages/sdk-alpha/scripts/validate-exports.mjspackages/sdk-alpha/src/index.tspackages/sdk-alpha/package.json
📚 Learning: 2025-08-01T17:55:02.423Z
Learnt from: transphorm
PR: selfxyz/self#823
File: app/ios/Podfile:108-116
Timestamp: 2025-08-01T17:55:02.423Z
Learning: In React Native iOS projects, the RNSentry target can exist and require warning suppressions (CLANG_WARN_NULLABLE_TO_NONNULL_CONVERSION, CLANG_WARN_OBJC_IMPLICIT_RETAIN_SELF, GCC_WARN_ABOUT_DEPRECATED_FUNCTIONS) even when Sentry npm packages aren't directly visible in package.json. The target is created through React Native Sentry integration and the warning suppressions are necessary and functional.
Applied to files:
app/package.json
📚 Learning: 2025-06-30T15:51:11.193Z
Learnt from: transphorm
PR: selfxyz/self#636
File: app/android/app/build.gradle:76-76
Timestamp: 2025-06-30T15:51:11.193Z
Learning: For React Native 0.80, the correct JSC version is `io.github.react-native-community:jsc-android:2026004.+`. This version was specifically built with Android NDK r23 to align with React Native 0.80's NDK requirements and prevent compatibility issues. The `2026004` version pattern is the official version for RN 0.80, not an outdated pin.
Applied to files:
app/package.json
📚 Learning: 2025-06-30T15:27:13.795Z
Learnt from: transphorm
PR: selfxyz/self#636
File: app/ios/Podfile:14-14
Timestamp: 2025-06-30T15:27:13.795Z
Learning: React Native 0.80 supports iOS 15.1 as the minimum deployment target, not iOS 16.0. This allows for broader device compatibility while still being compatible with the React Native 0.80 upgrade.
Applied to files:
app/package.json
📚 Learning: 2025-08-01T17:11:39.232Z
Learnt from: transphorm
PR: selfxyz/self#823
File: common/package.json:21-60
Timestamp: 2025-08-01T17:11:39.232Z
Learning: Explicit export mappings in package.json exports field provide better tree shaking than wildcard patterns because they enable easier static analysis by bundlers, allowing for more precise dependency graph understanding and better dead code elimination.
Applied to files:
package.jsonapp/tsconfig.jsonpackages/sdk-alpha/scripts/verify-conditions.mjspackages/sdk-alpha/tsconfig.jsonpackages/sdk-alpha/src/adapters/index.tspackages/sdk-alpha/scripts/validate-exports.mjspackages/sdk-alpha/src/index.tspackages/sdk-alpha/package.json
🪛 GitHub Actions: Web CI
package.json
[warning] 14-15: The condition "types" will never be used as it comes after "default" in package.json exports.
packages/sdk-alpha/package.json
[warning] 14-15: The condition "types" will never be used as it comes after "default" in package.json exports.
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e-ios
- GitHub Check: analyze-android
- GitHub Check: analyze-ios
- GitHub Check: build
🔇 Additional comments (16)
app/src/components/native/PassportCamera.web.tsx (1)
5-5: Clean import migration to SDK package.The type-only import from
@selfxyz/sdk-alphais properly implemented. Since this is a web stub implementation, importing only the type for the callback signature is appropriate and efficient.app/src/screens/passport/PassportCameraScreen.tsx (1)
8-8: Import migration looks good.The
formatDateToYYMMDDfunction import has been successfully migrated to the SDK package. The usage remains consistent with the existing iOS-specific date formatting logic.app/package.json (1)
88-88: Proper workspace dependency addition.The
@selfxyz/sdk-alphadependency withworkspace:^specifier is correctly configured for the monorepo structure.app/src/components/native/PassportCamera.tsx (1)
4-11: Well-organized import migration.The React Native imports have been cleanly consolidated and the
extractMRZInfofunction has been properly migrated to the SDK package. The function usage remains consistent within the native event handling logic.app/tsconfig.json (1)
11-12: LGTM! Proper TypeScript path mapping for the new SDK package.The dual path mapping for
@selfxyz/sdk-alphacorrectly points to both source and distribution directories, enabling seamless development and build workflows. This follows the established pattern and integrates well with the monorepo structure.packages/sdk-alpha/src/config/defaults.ts (1)
3-8: Solid default configuration with appropriate timeout values.The timeout configurations are well-balanced for mobile environments (30s HTTP, 60s WebSocket/scan, 120s proof generation). TLS pinning disabled by default is acceptable for an alpha SDK, but consider enabling it for production releases to enhance security.
packages/sdk-alpha/src/adapters/index.ts (1)
1-4: Excellent use of explicit exports for better tree-shaking.The explicit named exports provide better static analysis capabilities for bundlers compared to wildcard patterns, enabling more precise dependency graphs and dead code elimination. The comprehensive adapter interface coverage supports good testability and separation of concerns.
packages/sdk-alpha/tsconfig.json (1)
2-18: Well-configured TypeScript setup for modern SDK development.The configuration uses appropriate modern settings: ES2020 target for good compatibility, bundler module resolution for optimal bundling, strict type checking for safety, and React JSX transform for efficiency. Declaration file generation ensures proper TypeScript consumption by consumers.
packages/sdk-alpha/scripts/verify-conditions.mjs (1)
1-16: LGTM: Well-structured package validation script.The validation logic correctly enforces critical ESM package requirements. The error collection approach allows reporting all issues at once rather than failing fast, which improves developer experience.
packages/sdk-alpha/scripts/validate-exports.mjs (1)
13-13: LGTM: Regex pattern correctly identifies default exports.The regex
/\bexport\s+default\b/properly uses word boundaries to avoid false matches and handles whitespace variations in export syntax.packages/sdk-alpha/src/index.ts (1)
1-6: LGTM: Clean ESM export structure.The re-export pattern effectively consolidates the public API surface. Using
.jsextensions in imports correctly follows ESM requirements for explicit file extensions.packages/sdk-alpha/package.json (2)
21-26: LGTM: Comprehensive build and validation pipeline.The build script correctly uses tsup for ESM output with declarations. The prepublish script runs thorough validation including export checks and package condition verification, ensuring quality before publishing.
5-6: LGTM: Correct ESM and tree-shaking configuration.Setting
"type": "module"and"sideEffects": falsecorrectly configures the package for optimal tree-shaking, aligning with the explicit export mappings strategy from previous learnings.packages/sdk-alpha/README.md (2)
7-9: LGTM: Important security considerations documented.The NFC lifecycle requirements and server auto-start warnings are critical security considerations properly documented for mobile app developers.
18-21: Import path consistency verifiedThe package name in packages/sdk-alpha/package.json matches
@selfxyz/sdk-alpha, the tsconfig paths map correctly to bothsrcanddist, and all existing imports inapp/src/use@selfxyz/sdk-alphaconsistently. No changes needed here.packages/sdk-alpha/src/processing/mrz.ts (1)
4-6: Potential ReDoS vulnerability in regex patternsThe complex regex patterns, especially
TD1_REGEX, contain nested quantifiers that could be exploited for ReDoS attacks with malicious input strings. Consider simplifying the patterns or adding input length validation.export function extractMRZInfo(mrzString: string) { + // Validate input length to prevent ReDoS attacks + if (mrzString.length > 200) { + throw new Error("MRZ string too long"); + } + const mrzLines = mrzString.split("\n");Likely an incorrect or invalid review comment.
| "circuits", | ||
| "common", | ||
| "contracts", | ||
| "packages/*", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Workspace configuration updated correctly.
The addition of "packages/*" to workspaces properly enables discovery of the new SDK package.
Address the pipeline warning about exports condition ordering.
The pipeline shows a warning about the "types" condition coming after "default" in package.json exports. This affects TypeScript resolution and should be fixed.
Based on the pipeline warning, please run the following script to identify and fix the exports condition ordering issue:
🏁 Script executed:
#!/bin/bash
# Description: Find package.json files with exports field and check condition ordering
# Expected: "types" should come before "default" in exports conditions
find . -name "package.json" -not -path "./node_modules/*" -exec grep -l '"exports"' {} \; | xargs grep -A 10 -B 2 '"exports"'Length of output: 2684
Workspace configuration updated correctly.
The addition of "packages/*" to the root workspaces array is necessary and correctly enables Yarn to discover the new SDK package.
Fix exports condition ordering in packages/sdk-alpha
The pipeline warning stems from the exports block in packages/sdk-alpha/package.json placing "types" after "default". To ensure proper module resolution (especially for TypeScript), move "types" above "default":
• packages/sdk-alpha/package.json
"exports": {
".": {
"react-native": "./dist/index.js",
"browser": "./dist/index.js",
- "default": "./dist/index.js",
- "types": "./dist/index.d.ts"
+ "types": "./dist/index.d.ts",
+ "default": "./dist/index.js"
}
}This aligns with best practices seen in your other packages (e.g., sdk/core, sdk/qrcode). Once updated, the pipeline warning will be resolved.
📝 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.
| "packages/*", | |
| "exports": { | |
| ".": { | |
| "react-native": "./dist/index.js", | |
| "browser": "./dist/index.js", | |
| "types": "./dist/index.d.ts", | |
| "default": "./dist/index.js" | |
| } | |
| } |
🤖 Prompt for AI Agents
In package.json at line 8, the workspace configuration is correct with
"packages/*". However, in packages/sdk-alpha/package.json, reorder the "exports"
block so that the "types" condition appears before the "default" condition. This
change ensures proper module resolution and aligns with best practices used in
other SDK packages, resolving the pipeline warning.
| const dist = new URL("../dist/", import.meta.url); | ||
| const files = await readdir(dist); | ||
| let hasDefault = false; | ||
|
|
||
| for (const f of files) { | ||
| if (!f.endsWith(".js")) continue; | ||
| const src = await readFile(join(dist.pathname, f), "utf8"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix mixed URL and path handling.
Mixing URL object with path.join can cause cross-platform issues. The dist is already a URL object, so use consistent URL handling.
-const dist = new URL("../dist/", import.meta.url);
-const files = await readdir(dist);
+const distUrl = new URL("../dist/", import.meta.url);
+const files = await readdir(distUrl);
let hasDefault = false;
for (const f of files) {
if (!f.endsWith(".js")) continue;
- const src = await readFile(join(dist.pathname, f), "utf8");
+ const src = await readFile(new URL(f, distUrl), "utf8");📝 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.
| const dist = new URL("../dist/", import.meta.url); | |
| const files = await readdir(dist); | |
| let hasDefault = false; | |
| for (const f of files) { | |
| if (!f.endsWith(".js")) continue; | |
| const src = await readFile(join(dist.pathname, f), "utf8"); | |
| const distUrl = new URL("../dist/", import.meta.url); | |
| const files = await readdir(distUrl); | |
| let hasDefault = false; | |
| for (const f of files) { | |
| if (!f.endsWith(".js")) continue; | |
| const src = await readFile(new URL(f, distUrl), "utf8"); |
🤖 Prompt for AI Agents
In packages/sdk-alpha/scripts/validate-exports.mjs around lines 6 to 12, the
code mixes URL objects with path.join, which can cause cross-platform issues.
Replace the use of path.join with URL methods to handle paths consistently as
URLs. For example, use new URL(f, dist) to construct file URLs instead of
joining paths, ensuring all file operations use URL objects uniformly.
| async scan(opts: ScanOpts & { signal?: AbortSignal }): Promise<ScanResult> { | ||
| switch (opts.mode) { | ||
| case "qr": | ||
| return { mode: "qr", data: "self://stub-qr" }; | ||
| case "mrz": | ||
| throw Object.assign(new Error("MRZ scan not supported in web shim"), { | ||
| code: "SELF_ERR_SCANNER_UNAVAILABLE", category: "scanner", retryable: false | ||
| }); | ||
| case "nfc": | ||
| throw Object.assign(new Error("NFC not supported in web shim"), { | ||
| code: "SELF_ERR_NFC_NOT_SUPPORTED", category: "scanner", retryable: false | ||
| }); | ||
| default: | ||
| throw Object.assign(new Error("Unknown scan mode"), { | ||
| code: "SELF_ERR_SCANNER_MODE", category: "scanner", retryable: false | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
AbortSignal not implemented for cancellation.
The method accepts an AbortSignal but doesn't use it for cancellation. For consistency and proper async operation handling, consider checking the signal status before operations.
async scan(opts: ScanOpts & { signal?: AbortSignal }): Promise<ScanResult> {
+ if (opts.signal?.aborted) {
+ throw Object.assign(new Error("Scan operation aborted"), {
+ code: "SELF_ERR_ABORTED", category: "scanner", retryable: false
+ });
+ }
+
switch (opts.mode) {📝 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.
| async scan(opts: ScanOpts & { signal?: AbortSignal }): Promise<ScanResult> { | |
| switch (opts.mode) { | |
| case "qr": | |
| return { mode: "qr", data: "self://stub-qr" }; | |
| case "mrz": | |
| throw Object.assign(new Error("MRZ scan not supported in web shim"), { | |
| code: "SELF_ERR_SCANNER_UNAVAILABLE", category: "scanner", retryable: false | |
| }); | |
| case "nfc": | |
| throw Object.assign(new Error("NFC not supported in web shim"), { | |
| code: "SELF_ERR_NFC_NOT_SUPPORTED", category: "scanner", retryable: false | |
| }); | |
| default: | |
| throw Object.assign(new Error("Unknown scan mode"), { | |
| code: "SELF_ERR_SCANNER_MODE", category: "scanner", retryable: false | |
| }); | |
| } | |
| } | |
| async scan(opts: ScanOpts & { signal?: AbortSignal }): Promise<ScanResult> { | |
| if (opts.signal?.aborted) { | |
| throw Object.assign(new Error("Scan operation aborted"), { | |
| code: "SELF_ERR_ABORTED", category: "scanner", retryable: false | |
| }); | |
| } | |
| switch (opts.mode) { | |
| case "qr": | |
| return { mode: "qr", data: "self://stub-qr" }; | |
| case "mrz": | |
| throw Object.assign(new Error("MRZ scan not supported in web shim"), { | |
| code: "SELF_ERR_SCANNER_UNAVAILABLE", category: "scanner", retryable: false | |
| }); | |
| case "nfc": | |
| throw Object.assign(new Error("NFC not supported in web shim"), { | |
| code: "SELF_ERR_NFC_NOT_SUPPORTED", category: "scanner", retryable: false | |
| }); | |
| default: | |
| throw Object.assign(new Error("Unknown scan mode"), { | |
| code: "SELF_ERR_SCANNER_MODE", category: "scanner", retryable: false | |
| }); | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/sdk-alpha/src/adapters/web/shims.ts between lines 4 and 21, the scan
method accepts an AbortSignal in its options but does not check or handle it for
cancellation. To fix this, add a check at the start of the method to see if the
signal is already aborted and throw an appropriate error if so. Also, set up a
listener on the signal to handle cancellation during the async operation,
ensuring the method respects the AbortSignal for proper cancellation behavior.
| throw Object.assign(new Error("MRZ scan not supported in web shim"), { | ||
| code: "SELF_ERR_SCANNER_UNAVAILABLE", category: "scanner", retryable: false | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Use consistent error creation utilities.
Consider using the SDK's error creation utilities from the errors module instead of Object.assign for more maintainable and consistent error handling.
Let me verify if there are error creation utilities available in the SDK:
🏁 Script executed:
#!/bin/bash
# Description: Check for error creation utilities in the SDK errors module
# Expected: Find error creation functions that could be used instead of Object.assign
fd -t f "errors" packages/sdk-alpha/src --exec cat {} \;Length of output: 500
Use the sdkError utility for consistent error creation
Replace the manual Object.assign call with the shared sdkError function to keep error instantiation uniform across the SDK.
• Import the helper at the top of packages/sdk-alpha/src/adapters/web/shims.ts
import { sdkError } from "../../errors";• Replace the throw site:
- throw Object.assign(
- new Error("MRZ scan not supported in web shim"),
- { code: "SELF_ERR_SCANNER_UNAVAILABLE", category: "scanner", retryable: false }
- );
+ throw sdkError(
+ "MRZ scan not supported in web shim",
+ "SELF_ERR_SCANNER_UNAVAILABLE",
+ "scanner",
+ false
+ );This ensures all SDK errors share the same shape and creation logic.
🤖 Prompt for AI Agents
In packages/sdk-alpha/src/adapters/web/shims.ts around lines 9 to 11, replace
the manual error creation using Object.assign with the shared sdkError utility
for consistent error handling. First, import sdkError from "../../errors" at the
top of the file. Then, update the throw statement to use sdkError with the same
message and properties (code, category, retryable) to ensure uniform error shape
across the SDK.
| return { | ||
| id: "stub", | ||
| status: "pending", | ||
| result: async () => ({ ok: false, reason: "SELF_ERR_PROOF_STUB" }), | ||
| cancel: () => {} | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve stub ProofHandle implementation
The current stub implementation could be misleading as it returns a "pending" status but never actually processes. Consider making the unimplemented state more explicit.
return {
id: "stub",
- status: "pending",
- result: async () => ({ ok: false, reason: "SELF_ERR_PROOF_STUB" }),
+ status: "failed",
+ result: async () => ({ ok: false, reason: "Proof generation not yet implemented" }),
cancel: () => {}
};🤖 Prompt for AI Agents
In packages/sdk-alpha/src/client.ts around lines 42 to 47, the stub ProofHandle
returns a "pending" status but does not perform any processing, which can be
misleading. Change the status to explicitly indicate an unimplemented or error
state, such as "error" or "unimplemented," and update the result function to
clearly reflect this state. This will make the stub's behavior more explicit and
avoid confusion.
| const isTD1 = TD1_REGEX.test(mrzLines[0]) || mrzLines[0]?.startsWith("I"); | ||
| const isTD3 = TD3_line_2_REGEX.test(mrzLines[1] || ""); | ||
| if (!isTD1 && !isTD3) { | ||
| throw new Error("Invalid MRZ format: expected TD1 or TD3"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve MRZ format detection logic
The current logic uses startsWith("I") as a fallback for TD1 detection, which is imprecise and could lead to false positives. Consider using more specific validation.
- const isTD1 = TD1_REGEX.test(mrzLines[0]) || mrzLines[0]?.startsWith("I");
- const isTD3 = TD3_line_2_REGEX.test(mrzLines[1] || "");
+ const isTD1 = mrzLines.length >= 1 && TD1_REGEX.test(mrzLines[0]);
+ const isTD3 = mrzLines.length >= 2 && TD3_line_2_REGEX.test(mrzLines[1]);🤖 Prompt for AI Agents
In packages/sdk-alpha/src/processing/mrz.ts around lines 8 to 12, the MRZ format
detection uses a loose check with startsWith("I") for TD1, which can cause false
positives. Replace this with a more precise validation by using a stricter regex
pattern or additional checks that accurately match the TD1 MRZ format instead of
just checking the first character.
| documentType = line.slice(0, 2).replace(/</g, "").trim(); | ||
| countryCode = line.slice(2, 5).replace(/</g, "").trim(); | ||
| passportNumber = line.slice(5, 14).replace(/</g, "").trim(); | ||
| dateOfBirth = line.slice(30, 36).trim(); | ||
| dateOfExpiry = line.slice(38, 44).trim(); | ||
| } else { | ||
| documentType = mrzLines[0].slice(0, 2).replace(/</g, "").trim(); | ||
| countryCode = mrzLines[0].slice(2, 5).replace(/</g, "").trim(); | ||
| passportNumber = mrzLines[1].slice(0, 9).replace(/</g, "").trim(); | ||
| dateOfBirth = mrzLines[1].slice(13, 19).trim(); | ||
| dateOfExpiry = mrzLines[1].slice(21, 27).trim(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract hardcoded positions to constants and validate extracted dates
The hardcoded slice positions make the code brittle and difficult to maintain. Additionally, extracted dates should be validated for correctness.
+// TD1 field positions
+const TD1_POSITIONS = {
+ DOCUMENT_TYPE: [0, 2],
+ COUNTRY_CODE: [2, 5],
+ PASSPORT_NUMBER: [5, 14],
+ DATE_OF_BIRTH: [30, 36],
+ DATE_OF_EXPIRY: [38, 44]
+} as const;
+
+// TD3 field positions
+const TD3_POSITIONS = {
+ DOCUMENT_TYPE: [0, 2],
+ COUNTRY_CODE: [2, 5],
+ PASSPORT_NUMBER: [0, 9],
+ DATE_OF_BIRTH: [13, 19],
+ DATE_OF_EXPIRY: [21, 27]
+} as const;
+
+function validateMRZDate(dateStr: string): boolean {
+ return /^\d{6}$/.test(dateStr) && dateStr !== '000000';
+}
if (isTD1) {
const line = mrzLines[0];
- documentType = line.slice(0, 2).replace(/</g, "").trim();
- countryCode = line.slice(2, 5).replace(/</g, "").trim();
- passportNumber = line.slice(5, 14).replace(/</g, "").trim();
- dateOfBirth = line.slice(30, 36).trim();
- dateOfExpiry = line.slice(38, 44).trim();
+ documentType = line.slice(...TD1_POSITIONS.DOCUMENT_TYPE).replace(/</g, "").trim();
+ countryCode = line.slice(...TD1_POSITIONS.COUNTRY_CODE).replace(/</g, "").trim();
+ passportNumber = line.slice(...TD1_POSITIONS.PASSPORT_NUMBER).replace(/</g, "").trim();
+ dateOfBirth = line.slice(...TD1_POSITIONS.DATE_OF_BIRTH).trim();
+ dateOfExpiry = line.slice(...TD1_POSITIONS.DATE_OF_EXPIRY).trim();
+
+ if (!validateMRZDate(dateOfBirth) || !validateMRZDate(dateOfExpiry)) {
+ throw new Error("Invalid date format in MRZ");
+ }
} else {
- documentType = mrzLines[0].slice(0, 2).replace(/</g, "").trim();
- countryCode = mrzLines[0].slice(2, 5).replace(/</g, "").trim();
- passportNumber = mrzLines[1].slice(0, 9).replace(/</g, "").trim();
- dateOfBirth = mrzLines[1].slice(13, 19).trim();
- dateOfExpiry = mrzLines[1].slice(21, 27).trim();
+ documentType = mrzLines[0].slice(...TD3_POSITIONS.DOCUMENT_TYPE).replace(/</g, "").trim();
+ countryCode = mrzLines[0].slice(...TD3_POSITIONS.COUNTRY_CODE).replace(/</g, "").trim();
+ passportNumber = mrzLines[1].slice(...TD3_POSITIONS.PASSPORT_NUMBER).replace(/</g, "").trim();
+ dateOfBirth = mrzLines[1].slice(...TD3_POSITIONS.DATE_OF_BIRTH).trim();
+ dateOfExpiry = mrzLines[1].slice(...TD3_POSITIONS.DATE_OF_EXPIRY).trim();
+
+ if (!validateMRZDate(dateOfBirth) || !validateMRZDate(dateOfExpiry)) {
+ throw new Error("Invalid date format in MRZ");
+ }
}📝 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.
| documentType = line.slice(0, 2).replace(/</g, "").trim(); | |
| countryCode = line.slice(2, 5).replace(/</g, "").trim(); | |
| passportNumber = line.slice(5, 14).replace(/</g, "").trim(); | |
| dateOfBirth = line.slice(30, 36).trim(); | |
| dateOfExpiry = line.slice(38, 44).trim(); | |
| } else { | |
| documentType = mrzLines[0].slice(0, 2).replace(/</g, "").trim(); | |
| countryCode = mrzLines[0].slice(2, 5).replace(/</g, "").trim(); | |
| passportNumber = mrzLines[1].slice(0, 9).replace(/</g, "").trim(); | |
| dateOfBirth = mrzLines[1].slice(13, 19).trim(); | |
| dateOfExpiry = mrzLines[1].slice(21, 27).trim(); | |
| } | |
| // TD1 field positions | |
| const TD1_POSITIONS = { | |
| DOCUMENT_TYPE: [0, 2], | |
| COUNTRY_CODE: [2, 5], | |
| PASSPORT_NUMBER: [5, 14], | |
| DATE_OF_BIRTH: [30, 36], | |
| DATE_OF_EXPIRY: [38, 44] | |
| } as const; | |
| // TD3 field positions | |
| const TD3_POSITIONS = { | |
| DOCUMENT_TYPE: [0, 2], | |
| COUNTRY_CODE: [2, 5], | |
| PASSPORT_NUMBER: [0, 9], | |
| DATE_OF_BIRTH: [13, 19], | |
| DATE_OF_EXPIRY: [21, 27] | |
| } as const; | |
| function validateMRZDate(dateStr: string): boolean { | |
| return /^\d{6}$/.test(dateStr) && dateStr !== '000000'; | |
| } | |
| if (isTD1) { | |
| const line = mrzLines[0]; | |
| documentType = line.slice(...TD1_POSITIONS.DOCUMENT_TYPE).replace(/</g, "").trim(); | |
| countryCode = line.slice(...TD1_POSITIONS.COUNTRY_CODE).replace(/</g, "").trim(); | |
| passportNumber = line.slice(...TD1_POSITIONS.PASSPORT_NUMBER).replace(/</g, "").trim(); | |
| dateOfBirth = line.slice(...TD1_POSITIONS.DATE_OF_BIRTH).trim(); | |
| dateOfExpiry = line.slice(...TD1_POSITIONS.DATE_OF_EXPIRY).trim(); | |
| if (!validateMRZDate(dateOfBirth) || !validateMRZDate(dateOfExpiry)) { | |
| throw new Error("Invalid date format in MRZ"); | |
| } | |
| } else { | |
| documentType = mrzLines[0].slice(...TD3_POSITIONS.DOCUMENT_TYPE).replace(/</g, "").trim(); | |
| countryCode = mrzLines[0].slice(...TD3_POSITIONS.COUNTRY_CODE).replace(/</g, "").trim(); | |
| passportNumber = mrzLines[1].slice(...TD3_POSITIONS.PASSPORT_NUMBER).replace(/</g, "").trim(); | |
| dateOfBirth = mrzLines[1].slice(...TD3_POSITIONS.DATE_OF_BIRTH).trim(); | |
| dateOfExpiry = mrzLines[1].slice(...TD3_POSITIONS.DATE_OF_EXPIRY).trim(); | |
| if (!validateMRZDate(dateOfBirth) || !validateMRZDate(dateOfExpiry)) { | |
| throw new Error("Invalid date format in MRZ"); | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/sdk-alpha/src/processing/mrz.ts around lines 17 to 28, the slice
positions used to extract fields from MRZ lines are hardcoded, making the code
brittle and hard to maintain. Define constants for these slice indices with
descriptive names to replace the magic numbers. Additionally, after extracting
dateOfBirth and dateOfExpiry, add validation logic to ensure these dates are in
the correct format and represent valid dates before proceeding.
| export function formatDateToYYMMDD(inputDate: string) { | ||
| const year = inputDate.substring(2, 4); | ||
| const month = inputDate.substring(5, 7); | ||
| const day = inputDate.substring(8, 10); | ||
| return year + month + day; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and improve documentation
The function assumes a specific input format but doesn't validate it, which could cause runtime errors. The function name also doesn't indicate the expected input format.
+/**
+ * Converts a date string from ISO-like format (YYYY-MM-DD) to YYMMDD format
+ * @param inputDate - Date string in YYYY-MM-DD format
+ * @returns Date string in YYMMDD format
+ * @throws Error if input format is invalid
+ */
-export function formatDateToYYMMDD(inputDate: string) {
+export function formatDateToYYMMDD(inputDate: string): string {
+ if (!inputDate || inputDate.length < 10) {
+ throw new Error("Invalid date format: expected YYYY-MM-DD");
+ }
+
+ // Validate basic format
+ if (!/^\d{4}-\d{2}-\d{2}/.test(inputDate)) {
+ throw new Error("Invalid date format: expected YYYY-MM-DD");
+ }
+
const year = inputDate.substring(2, 4);
const month = inputDate.substring(5, 7);
const day = inputDate.substring(8, 10);
return year + month + day;
}📝 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.
| export function formatDateToYYMMDD(inputDate: string) { | |
| const year = inputDate.substring(2, 4); | |
| const month = inputDate.substring(5, 7); | |
| const day = inputDate.substring(8, 10); | |
| return year + month + day; | |
| } | |
| /** | |
| * Converts a date string from ISO-like format (YYYY-MM-DD) to YYMMDD format | |
| * @param inputDate - Date string in YYYY-MM-DD format | |
| * @returns Date string in YYMMDD format | |
| * @throws Error if input format is invalid | |
| */ | |
| export function formatDateToYYMMDD(inputDate: string): string { | |
| if (!inputDate || inputDate.length < 10) { | |
| throw new Error("Invalid date format: expected YYYY-MM-DD"); | |
| } | |
| // Validate basic format | |
| if (!/^\d{4}-\d{2}-\d{2}/.test(inputDate)) { | |
| throw new Error("Invalid date format: expected YYYY-MM-DD"); | |
| } | |
| const year = inputDate.substring(2, 4); | |
| const month = inputDate.substring(5, 7); | |
| const day = inputDate.substring(8, 10); | |
| return year + month + day; | |
| } |
🤖 Prompt for AI Agents
In packages/sdk-alpha/src/processing/mrz.ts around lines 32 to 37, the function
formatDateToYYMMDD lacks input validation and has an unclear name regarding the
expected input format. Add validation to check that inputDate matches the
expected date format (e.g., YYYY-MM-DD) before processing to prevent runtime
errors. Rename the function to reflect the expected input format clearly, and
update or add documentation comments to specify the input format and behavior.
| export type ScanResult = | ||
| | { mode: "mrz"; passportNumber: string; dateOfBirth: string; dateOfExpiry: string; countryCode?: string } | ||
| | { mode: "nfc"; raw: unknown } | ||
| | { mode: "qr"; data: string }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance MRZ scan result type with validation
The MRZ scan result type should include validation to ensure data integrity, especially since this handles sensitive passport information.
export type ScanResult =
- | { mode: "mrz"; passportNumber: string; dateOfBirth: string; dateOfExpiry: string; countryCode?: string }
+ | {
+ mode: "mrz";
+ passportNumber: string;
+ dateOfBirth: string;
+ dateOfExpiry: string;
+ countryCode?: string;
+ documentType?: string;
+ validated: boolean;
+ }
| { mode: "nfc"; raw: unknown }
| { mode: "qr"; data: string };Adding documentType and validated fields improves type safety and aligns with the actual extractMRZInfo function output.
📝 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.
| export type ScanResult = | |
| | { mode: "mrz"; passportNumber: string; dateOfBirth: string; dateOfExpiry: string; countryCode?: string } | |
| | { mode: "nfc"; raw: unknown } | |
| | { mode: "qr"; data: string }; | |
| export type ScanResult = | |
| | { | |
| mode: "mrz"; | |
| passportNumber: string; | |
| dateOfBirth: string; | |
| dateOfExpiry: string; | |
| countryCode?: string; | |
| documentType?: string; | |
| validated: boolean; | |
| } | |
| | { mode: "nfc"; raw: unknown } | |
| | { mode: "qr"; data: string }; |
🤖 Prompt for AI Agents
In packages/sdk-alpha/src/types/public.ts around lines 7 to 10, enhance the
ScanResult type for the "mrz" mode by adding the fields documentType and
validated to better reflect the actual output of the extractMRZInfo function.
This involves extending the MRZ object type with these two fields to improve
type safety and ensure validation status is captured for sensitive passport
data.
|
@shazarre @aaronmgdr this lays the foundational work for our alpha sdk. my idea here is that we create a local monorepo package to import from as a mock before creating the real package. plan on merging this once it gets to a decent alpha state. please feel free to critique and rewrite major pieces to improve implementation. |
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (9)
app/tests/e2e/launch.android.flow.yaml (1)
6-6: Doc + guardrail suggestion for timeouts.
- Document LAUNCH_WAIT_MS in the E2E README and CI envs.
- Optionally allow platform overrides (e.g., LAUNCH_WAIT_ANDROID_MS) falling back to LAUNCH_WAIT_MS.
- Consider a sanity upper bound in CI (e.g., 180000) to avoid excessive hangs.
Happy to propose doc updates or a tiny script to validate the env var before running Maestro.
packages/sdk-alpha/.eslintrc.cjs (4)
28-39: Tighten simple-import-sort groups (remove redundant match; add side-effect group)'^node:.*/' is shadowed by '^node:' and never matches. Add a side-effect group and conventional ordering to reduce churn.
'simple-import-sort/imports': [ 'error', { groups: [ - ['^node:'], - ['^node:.*/'], - ['^[a-zA-Z]'], - ['^@selfxyz/'], - ['^[./]'], + // Side-effect imports (e.g., polyfills) + ['^\\u0000'], + // Node core + ['^node:'], + // Third-party packages + ['^@?\\w'], + // Internal scoped packages + ['^@selfxyz/'], + // Relative imports + ['^[./]'], ], }, ],
48-48: Prefer warning on explicit any to keep type-safety signals without blockingFully disabling any hides debt and potential runtime risks. A warning provides visibility without friction.
- '@typescript-eslint/no-explicit-any': 'off', + '@typescript-eslint/no-explicit-any': 'warn',
54-71: Enforce named exports in library source for better tree-shakingGiven the emphasis on explicit exports mappings (see prior learnings), disallow default exports in src. This improves treeshaking and API clarity.
overrides: [ { files: ['*.cjs'], env: { node: true, commonjs: true, es6: true, }, parserOptions: { ecmaVersion: 'latest', sourceType: 'script', }, rules: { '@typescript-eslint/no-var-requires': 'off', }, }, + { + files: ['src/**/*.{ts,tsx}'], + rules: { + 'import/no-default-export': 'error', + }, + }, ],
16-16: Ignore coverage artifacts to reduce lint noise and CI timeGenerated coverage reports shouldn’t be linted.
- ignorePatterns: ['dist/', 'node_modules/'], + ignorePatterns: ['dist/', 'node_modules/', 'coverage/'],packages/sdk-alpha/scripts/report-exports.mjs (4)
5-5: Deterministic output: sort directory listing.Stable ordering improves reproducibility in CI logs.
-const files = await readdir(dist); +const files = (await readdir(dist)).sort();
19-21: Deterministic printing of report entries.Relying on object insertion order is okay for non-integer keys, but being explicit avoids surprises.
console.log('Exported symbols by file:'); -for (const [file, names] of Object.entries(report)) { - console.log(`- ${file}: ${names.join(', ') || '(none)'}`); -} +for (const file of Object.keys(report).sort()) { + const names = report[file] ?? []; + console.log(`- ${file}: ${names.join(', ') || '(none)'}`); +}
8-17: Resilience: handle per-file read/parse errors without crashing the whole run.A single malformed/empty artifact shouldn’t abort the entire report. Wrap file processing in try/catch and continue.
for (const f of files) { if (!f.endsWith('.js')) continue; - const src = await readFile(new URL(f, dist), 'utf8'); - const directVars = [...src.matchAll(/export\s+(?:const|let|var)\s+([$\w]+)/g)].map(m => m[1]); - const directFns = [...src.matchAll(/export\s+(?:async\s+)?function\s+([$\w]+)/g)].map(m => m[1]); - const directClasses = [...src.matchAll(/export\s+class\s+([$\w]+)/g)].map(m => m[1]); - const named = [...src.matchAll(/export\s*{([^}]+)}/g)] - .flatMap(m => m[1].split(',').map(s => s.trim()).filter(Boolean)) - .map(s => { - const m = s.match(/^([$\w]+)(?:\s+as\s+([$\w]+))?$/); - if (!m) return null; - const [, orig, alias] = m; - const name = alias || orig; - return name === 'default' ? null : name; // exclude final 'default' export - }) - .filter(Boolean); - report[f] = Array.from(new Set([...directVars, ...directFns, ...directClasses, ...named])); + try { + const src = await readFile(new URL(f, dist), 'utf8'); + const directVars = [...src.matchAll(/export\s+(?:const|let|var)\s+([$\w]+)/g)].map(m => m[1]); + const directFns = [...src.matchAll(/export\s+(?:async\s+)?function\s+([$\w]+)/g)].map(m => m[1]); + const directClasses = [...src.matchAll(/export\s+class\s+([$\w]+)/g)].map(m => m[1]); + const named = [...src.matchAll(/export\s*{([^}]+)}/g)] + .flatMap(m => m[1].split(',').map(s => s.trim()).filter(Boolean)) + .map(s => { + const m = s.match(/^([$\w]+)(?:\s+as\s+([$\w]+))?$/); + if (!m) return null; + const [, orig, alias] = m; + const name = alias || orig; + return name === 'default' ? null : name; + }) + .filter(Boolean); + report[f] = Array.from(new Set([...directVars, ...directFns, ...directClasses, ...named])); + } catch (err) { + console.warn(`Skipping ${f}: ${err?.message ?? err}`); + } }
1-22: Optional: use a proper ESM parser for complete and robust export detection.Regex will miss/fragilely parse cases like:
export * as ns from './x.js'export {foo} from './x.js'with complex spacing/comments- TypeScript emit edge-cases and minified outputs
Using es-module-lexer leads to correctness and future-proofing, and aligns with the learning on explicit export mappings improving tree-shaking.
Example (alternative implementation):
import { readdir, readFile } from 'node:fs/promises'; import { init, parse } from 'es-module-lexer'; const dist = new URL('../dist/', import.meta.url); const files = (await readdir(dist)).sort(); await init; const report = {}; for (const f of files) { if (!f.endsWith('.js')) continue; const src = await readFile(new URL(f, dist), 'utf8'); const [, exports] = parse(src); // exports includes named, star, and re-exports; filter 'default' report[f] = [...new Set(exports.filter(e => e !== 'default'))]; } for (const file of Object.keys(report).sort()) { const names = report[file] ?? []; console.log(`- ${file}: ${names.join(', ') || '(none)'}`); }This will also detect
export * as nsand re-exports reliably without brittle regexes.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
app/package.json(1 hunks)app/src/components/native/PassportCamera.tsx(1 hunks)app/src/screens/passport/PassportCameraScreen.tsx(2 hunks)app/tests/e2e/launch.android.flow.yaml(1 hunks)app/tests/e2e/launch.ios.flow.yaml(1 hunks)app/tsconfig.json(1 hunks)package.json(2 hunks)packages/sdk-alpha/.eslintrc.cjs(1 hunks)packages/sdk-alpha/.prettierignore(1 hunks)packages/sdk-alpha/.prettierrc(1 hunks)packages/sdk-alpha/MIGRATION_CHECKLIST.md(1 hunks)packages/sdk-alpha/README.md(1 hunks)packages/sdk-alpha/package.json(1 hunks)packages/sdk-alpha/scripts/report-exports.mjs(1 hunks)packages/sdk-alpha/src/client.ts(1 hunks)packages/sdk-alpha/tests/mrz.test.ts(1 hunks)packages/sdk-alpha/vitest.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- app/src/components/native/PassportCamera.tsx
- app/tests/e2e/launch.ios.flow.yaml
- packages/sdk-alpha/vitest.config.ts
- packages/sdk-alpha/.prettierrc
- packages/sdk-alpha/.prettierignore
- packages/sdk-alpha/MIGRATION_CHECKLIST.md
🚧 Files skipped from review as they are similar to previous changes (7)
- app/src/screens/passport/PassportCameraScreen.tsx
- app/package.json
- package.json
- app/tsconfig.json
- packages/sdk-alpha/README.md
- packages/sdk-alpha/package.json
- packages/sdk-alpha/src/client.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{test,spec}.{ts,js,tsx,jsx}
⚙️ CodeRabbit Configuration File
**/*.{test,spec}.{ts,js,tsx,jsx}: Review test files for:
- Test coverage completeness
- Test case quality and edge cases
- Mock usage appropriateness
- Test readability and maintainability
Files:
packages/sdk-alpha/tests/mrz.test.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: transphorm
PR: selfxyz/self#795
File: app/src/screens/prove/ProveScreen.tsx:5-5
Timestamp: 2025-07-28T19:18:48.270Z
Learning: The import path 'selfxyz/common/utils/scope' is valid when the common package has a "./utils/*" export mapping in package.json, even if ESLint shows resolution errors before the package is built. The package.json exports field properly maps "./utils/*" to the compiled files in dist/.
📚 Learning: 2025-08-01T17:11:39.232Z
Learnt from: transphorm
PR: selfxyz/self#823
File: common/package.json:21-60
Timestamp: 2025-08-01T17:11:39.232Z
Learning: Explicit export mappings in package.json exports field provide better tree shaking than wildcard patterns because they enable easier static analysis by bundlers, allowing for more precise dependency graph understanding and better dead code elimination.
Applied to files:
packages/sdk-alpha/scripts/report-exports.mjspackages/sdk-alpha/.eslintrc.cjs
📚 Learning: 2025-07-28T19:18:48.270Z
Learnt from: transphorm
PR: selfxyz/self#795
File: app/src/screens/prove/ProveScreen.tsx:5-5
Timestamp: 2025-07-28T19:18:48.270Z
Learning: The import path 'selfxyz/common/utils/scope' is valid when the common package has a "./utils/*" export mapping in package.json, even if ESLint shows resolution errors before the package is built. The package.json exports field properly maps "./utils/*" to the compiled files in dist/.
Applied to files:
packages/sdk-alpha/scripts/report-exports.mjspackages/sdk-alpha/.eslintrc.cjs
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: analyze-ios
- GitHub Check: analyze-android
- GitHub Check: e2e-ios
- GitHub Check: build
🔇 Additional comments (1)
app/tests/e2e/launch.android.flow.yaml (1)
6-6: Good shift to env-configurable timeout.Making the launch wait configurable via LAUNCH_WAIT_MS with a sane default improves test stability across environments.
| - extendedWaitUntil: | ||
| visible: "I have a Passport or Biometric ID" | ||
| timeout: 60000 | ||
| timeout: ${LAUNCH_WAIT_MS:60000} |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify iOS parity and CI wiring; ensure numeric values only.
- Confirm launch.ios.flow.yaml uses the same var.
- Ensure CI/local scripts export LAUNCH_WAIT_MS and it’s strictly numeric (milliseconds). Non-numeric values (e.g., "60s") can cause flaky behavior.
Run this script to verify parity and wiring:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Check iOS parity:"
fd -a 'launch.ios.flow.yaml' || true
rg -n --context 2 $'timeout: \\$\\{LAUNCH_WAIT_MS:60000\\}' --iglob 'app/tests/e2e/launch.ios.flow.yaml' || echo "Note: iOS file not found or not updated."
echo
echo "Scan for LAUNCH_WAIT_MS usage across repo:"
rg -n --heading 'LAUNCH_WAIT_MS' || true
echo
echo "Check common CI locations for LAUNCH_WAIT_MS export:"
fd -a '.github/workflows' | xargs -r rg -n 'LAUNCH_WAIT_MS' || true
fd -a 'Makefile|makefile|package.json|yarn.*|pnpm-.*|scripts|.env*' | xargs -r rg -n 'LAUNCH_WAIT_MS' || true
echo
echo "Tip: Keep LAUNCH_WAIT_MS numeric (e.g., 60000)."Length of output: 1539
Ensure CI exports a numeric LAUNCH_WAIT_MS
- ✅ Android and iOS flows are in sync (both use
timeout: ${LAUNCH_WAIT_MS:60000}at line 6 of their respective YAMLs). - ❌ I didn’t find any
LAUNCH_WAIT_MSexports in.github/workflows, Makefile, package.json, common scripts or env files. - Action: Add an explicit, strictly numeric export (for example,
) in your CI pipeline or local bootstrap script to avoid relying solely on the default and prevent flaky timeouts.
export LAUNCH_WAIT_MS=60000
🤖 Prompt for AI Agents
In app/tests/e2e/launch.android.flow.yaml at line 6, the timeout uses the
environment variable LAUNCH_WAIT_MS with a default of 60000, but this variable
is not explicitly exported in the CI or local environment. To fix this, add a
numeric export of LAUNCH_WAIT_MS (e.g., export LAUNCH_WAIT_MS=60000) in your CI
pipeline configuration or local bootstrap scripts to ensure the variable is
always defined and prevent flaky timeouts.
| module.exports = { | ||
| root: true, | ||
| parser: '@typescript-eslint/parser', | ||
| parserOptions: { | ||
| ecmaVersion: 2021, | ||
| sourceType: 'module', | ||
| ecmaFeatures: { jsx: false }, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make parserOptions monorepo-safe; future-proof ecmaVersion; enable unused-disable detection
Without tsconfigRootDir, running ESLint from the repo root often fails to locate ./tsconfig.json. Also, use 'latest' to avoid future syntax false-positives, and enable reportUnusedDisableDirectives for hygiene.
module.exports = {
root: true,
+ reportUnusedDisableDirectives: true,
parser: '@typescript-eslint/parser',
parserOptions: {
- ecmaVersion: 2021,
+ ecmaVersion: 'latest',
+ tsconfigRootDir: __dirname,
sourceType: 'module',
ecmaFeatures: { jsx: false },
},📝 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.
| module.exports = { | |
| root: true, | |
| parser: '@typescript-eslint/parser', | |
| parserOptions: { | |
| ecmaVersion: 2021, | |
| sourceType: 'module', | |
| ecmaFeatures: { jsx: false }, | |
| }, | |
| module.exports = { | |
| root: true, | |
| reportUnusedDisableDirectives: true, | |
| parser: '@typescript-eslint/parser', | |
| parserOptions: { | |
| ecmaVersion: 'latest', | |
| tsconfigRootDir: __dirname, | |
| sourceType: 'module', | |
| ecmaFeatures: { jsx: false }, | |
| }, |
🤖 Prompt for AI Agents
In packages/sdk-alpha/.eslintrc.cjs lines 1 to 8, update parserOptions to
include tsconfigRootDir set to __dirname to ensure ESLint correctly locates
tsconfig.json when run from the monorepo root. Change ecmaVersion from 2021 to
'latest' to support future ECMAScript syntax. Also, add
reportUnusedDisableDirectives: true to enable detection of unused ESLint disable
comments for better code hygiene.
| settings: { | ||
| 'import/resolver': { | ||
| typescript: { | ||
| alwaysTryTypes: true, | ||
| project: './tsconfig.json', | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add node resolver alongside typescript to stabilize resolution for .cjs/.js and built artifacts
The import plugin benefits from combining node and typescript resolvers in monorepos; this prevents false “unresolved” on CommonJS and non-TS imports.
settings: {
'import/resolver': {
+ node: {
+ extensions: ['.js', '.cjs', '.mjs', '.ts', '.tsx', '.d.ts'],
+ },
typescript: {
alwaysTryTypes: true,
project: './tsconfig.json',
},
},
},📝 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.
| settings: { | |
| 'import/resolver': { | |
| typescript: { | |
| alwaysTryTypes: true, | |
| project: './tsconfig.json', | |
| }, | |
| }, | |
| }, | |
| settings: { | |
| 'import/resolver': { | |
| node: { | |
| extensions: ['.js', '.cjs', '.mjs', '.ts', '.tsx', '.d.ts'], | |
| }, | |
| typescript: { | |
| alwaysTryTypes: true, | |
| project: './tsconfig.json', | |
| }, | |
| }, | |
| }, |
🤖 Prompt for AI Agents
In packages/sdk-alpha/.eslintrc.cjs around lines 17 to 24, the import resolver
configuration only includes the typescript resolver, which can cause false
unresolved errors for .cjs, .js, and built artifacts. To fix this, add the node
resolver alongside the typescript resolver under 'import/resolver' settings.
This means configuring both 'node' and 'typescript' resolvers together to
improve module resolution stability in the monorepo.
| 'import/first': 'error', | ||
| 'import/newline-after-import': 'error', | ||
| 'import/no-duplicates': 'error', | ||
| 'import/export': 'off', | ||
| '@typescript-eslint/no-explicit-any': 'off', | ||
| '@typescript-eslint/no-unused-vars': [ | ||
| 'error', | ||
| { argsIgnorePattern: '^_', varsIgnorePattern: '^_' }, | ||
| ], | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Re-enable key import rules and add common library safeguards
Turning off import/export drops helpful validations. Also add exports sorting and guard against undeclared deps (monorepo safety).
'import/first': 'error',
'import/newline-after-import': 'error',
- 'import/no-duplicates': 'error',
- 'import/export': 'off',
+ 'import/no-duplicates': 'error',
+ 'import/export': 'error',
+ 'simple-import-sort/exports': 'error',
+ 'import/no-extraneous-dependencies': [
+ 'error',
+ {
+ devDependencies: [
+ '**/*.test.*',
+ '**/*.spec.*',
+ 'tests/**',
+ '**/scripts/**',
+ '**/*.config.*',
+ ],
+ },
+ ],
'@typescript-eslint/no-explicit-any': 'off',
'@typescript-eslint/no-unused-vars': [
'error',
{ argsIgnorePattern: '^_', varsIgnorePattern: '^_' },
],📝 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.
| 'import/first': 'error', | |
| 'import/newline-after-import': 'error', | |
| 'import/no-duplicates': 'error', | |
| 'import/export': 'off', | |
| '@typescript-eslint/no-explicit-any': 'off', | |
| '@typescript-eslint/no-unused-vars': [ | |
| 'error', | |
| { argsIgnorePattern: '^_', varsIgnorePattern: '^_' }, | |
| ], | |
| }, | |
| 'import/first': 'error', | |
| 'import/newline-after-import': 'error', | |
| 'import/no-duplicates': 'error', | |
| 'import/export': 'error', | |
| 'simple-import-sort/exports': 'error', | |
| 'import/no-extraneous-dependencies': [ | |
| 'error', | |
| { | |
| devDependencies: [ | |
| '**/*.test.*', | |
| '**/*.spec.*', | |
| 'tests/**', | |
| '**/scripts/**', | |
| '**/*.config.*', | |
| ], | |
| }, | |
| ], | |
| '@typescript-eslint/no-explicit-any': 'off', | |
| '@typescript-eslint/no-unused-vars': [ | |
| 'error', | |
| { argsIgnorePattern: '^_', varsIgnorePattern: '^_' }, | |
| ], |
🤖 Prompt for AI Agents
In packages/sdk-alpha/.eslintrc.cjs around lines 44 to 53, re-enable the
'import/export' rule to restore important import/export validations.
Additionally, add rules for sorting exports and preventing undeclared
dependencies to improve code safety in the monorepo context. Update the ESLint
configuration accordingly to include these import-related safeguards.
| overrides: [ | ||
| { | ||
| files: ['*.cjs'], | ||
| env: { | ||
| node: true, | ||
| commonjs: true, | ||
| es6: true, | ||
| }, | ||
| parserOptions: { | ||
| ecmaVersion: 2020, | ||
| sourceType: 'script', | ||
| }, | ||
| rules: { | ||
| '@typescript-eslint/no-var-requires': 'off', | ||
| 'no-undef': 'off', | ||
| }, | ||
| }, | ||
| ], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Tighten .cjs override: avoid disabling no-undef; align ecmaVersion
no-undef catches real errors; with env.commonjs, require/exports are defined. Use latest syntax for parity.
env: {
node: true,
commonjs: true,
es6: true,
},
parserOptions: {
- ecmaVersion: 2020,
+ ecmaVersion: 'latest',
sourceType: 'script',
},
rules: {
- '@typescript-eslint/no-var-requires': 'off',
- 'no-undef': 'off',
+ '@typescript-eslint/no-var-requires': 'off',
},📝 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.
| overrides: [ | |
| { | |
| files: ['*.cjs'], | |
| env: { | |
| node: true, | |
| commonjs: true, | |
| es6: true, | |
| }, | |
| parserOptions: { | |
| ecmaVersion: 2020, | |
| sourceType: 'script', | |
| }, | |
| rules: { | |
| '@typescript-eslint/no-var-requires': 'off', | |
| 'no-undef': 'off', | |
| }, | |
| }, | |
| ], | |
| overrides: [ | |
| { | |
| files: ['*.cjs'], | |
| env: { | |
| node: true, | |
| commonjs: true, | |
| es6: true, | |
| }, | |
| parserOptions: { | |
| ecmaVersion: 'latest', | |
| sourceType: 'script', | |
| }, | |
| rules: { | |
| '@typescript-eslint/no-var-requires': 'off', | |
| }, | |
| }, | |
| ], |
🤖 Prompt for AI Agents
In packages/sdk-alpha/.eslintrc.cjs between lines 54 and 71, the override for
.cjs files disables the no-undef rule and sets ecmaVersion to 2020. To fix this,
remove the 'no-undef': 'off' rule to enable detection of undefined variables,
and update ecmaVersion in parserOptions to the latest version (e.g., 2024 or
'latest') to align with current syntax standards while keeping env.commonjs true
for proper Node.js CommonJS globals.
💡 Verification agent
🧩 Analysis chain
Add a tests override to enable test globals and avoid false positives
Define test env to prevent undefined globals (describe/it/test), and keep rules consistent.
overrides: [
{
files: ['*.cjs'],
env: {
node: true,
commonjs: true,
es6: true,
},
parserOptions: {
ecmaVersion: 'latest',
sourceType: 'script',
},
rules: {
'@typescript-eslint/no-var-requires': 'off',
},
},
+ {
+ files: ['**/*.test.ts', '**/*.spec.ts', 'tests/**/*.ts'],
+ env: { jest: true },
+ },
],Run this to verify the presence of the required ESLint plugins and detect the test runner for @selfxyz/sdk-alpha:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the sdk-alpha package.json
PKG_JSON="$(rg -n -S '"name":\\s*"@selfxyz/sdk-alpha"' -g '**/package.json' | head -n1 | cut -d: -f1)"
if [[ -z "${PKG_JSON:-}" ]]; then
echo "Could not find @selfxyz/sdk-alpha/package.json" >&2
exit 1
fi
echo "Using package.json at: $PKG_JSON"
echo -e "\nDev dependencies:"
jq -r '.devDependencies // {}' "$PKG_JSON"
echo -e "\nCheck required ESLint deps:"
REQ_DEPS='@typescript-eslint/parser @typescript-eslint/eslint-plugin eslint-plugin-import eslint-import-resolver-typescript eslint-import-resolver-node eslint-plugin-simple-import-sort'
for dep in $REQ_DEPS; do
jq -e --arg d "$dep" '.devDependencies[$d] // .dependencies[$d]' "$PKG_JSON" >/dev/null \
&& echo "✓ $dep found" || echo "✗ $dep missing"
done
echo -e "\nDetect test runner:"
jq -r '.devDependencies.jest // empty' "$PKG_JSON" >/dev/null && echo "Found jest in devDependencies"
jq -r '.devDependencies.vitest // empty' "$PKG_JSON" >/dev/null && echo "Found vitest in devDependencies"
# Check tsconfig existence (for resolver and parserOptions.project if later enabled)
PKG_DIR="$(dirname "$PKG_JSON")"
if [[ -f "$PKG_DIR/tsconfig.json" ]]; then
echo "✓ tsconfig.json present at $PKG_DIR/tsconfig.json"
else
echo "✗ tsconfig.json missing at $PKG_DIR"
fiLength of output: 125
To pinpoint where @selfxyz/sdk-alpha lives and verify its package.json, let’s locate the sdk-alpha folder and any matching package.json files:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1. Find the sdk-alpha directory:"
find . -type d -name "sdk-alpha" -print
echo -e "\n2. List all package.json files (to see where your packages are defined):"
find . -type f -name "package.json" -print
echo -e "\n3. Search for the package name inside those package.json files:"
rg -n '"@selfxyz/sdk-alpha"' -g '*/package.json' || echo "→ No occurrences of @selfxyz/sdk-alpha found in package.json"Length of output: 859
Let’s inspect the packages/sdk-alpha/package.json directly and check for ESLint/test deps and tsconfig:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
PKG_DIR="packages/sdk-alpha"
echo "1. package.json name & devDependencies:"
jq '{ name, devDependencies }' "$PKG_DIR/package.json"
echo -e "\n2. ESLint-related deps present?"
for dep in @typescript-eslint/parser @typescript-eslint/eslint-plugin eslint-plugin-import eslint-import-resolver-typescript eslint-import-resolver-node eslint-plugin-simple-import-sort; do
if jq -e --arg d "$dep" '.devDependencies[$d] // .dependencies[$d]' "$PKG_DIR/package.json" >/dev/null; then
echo "✓ $dep"
else
echo "✗ $dep"
fi
done
echo -e "\n3. Test runner deps (jest/vitest):"
for runner in jest vitest; do
if jq -e --arg r "$runner" '.devDependencies[$r]' "$PKG_DIR/package.json" >/dev/null; then
echo "✓ Found $runner"
fi
done
echo -e "\n4. tsconfig.json exists?"
if [[ -f "$PKG_DIR/tsconfig.json" ]]; then
echo "✓ tsconfig.json present"
else
echo "✗ tsconfig.json missing"
fiLength of output: 3436
Add test globals override to avoid undefined describe/it errors
Your packages/sdk-alpha/.eslintrc.cjs currently only sets up a Node/CommonJS environment, so ESLint will flag Jest-style globals (describe, it, test) as undefined. Since you’re using Vitest (which is Jest-compatible), add a test override:
overrides: [
{
files: ['*.cjs'],
env: {
node: true,
commonjs: true,
es6: true,
},
parserOptions: {
ecmaVersion: 2020,
sourceType: 'script',
},
rules: {
'@typescript-eslint/no-var-requires': 'off',
'no-undef': 'off',
},
},
+ {
+ files: ['**/*.{test,spec}.{ts,tsx,js,jsx}', 'tests/**/*.{ts,tsx,js,jsx}'],
+ env: {
+ jest: true, // Vitest runs Jest-style tests
+ },
+ },
],• If you’d rather use Vitest’s native env, install eslint-plugin-vitest and switch to env: { 'vitest/globals': true }.
• Note: eslint-import-resolver-typescript and eslint-import-resolver-node aren’t in your devDependencies; add them (and configure settings['import/resolver']) if you hit import-resolution errors.
📝 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.
| overrides: [ | |
| { | |
| files: ['*.cjs'], | |
| env: { | |
| node: true, | |
| commonjs: true, | |
| es6: true, | |
| }, | |
| parserOptions: { | |
| ecmaVersion: 2020, | |
| sourceType: 'script', | |
| }, | |
| rules: { | |
| '@typescript-eslint/no-var-requires': 'off', | |
| 'no-undef': 'off', | |
| }, | |
| }, | |
| ], | |
| overrides: [ | |
| { | |
| files: ['*.cjs'], | |
| env: { | |
| node: true, | |
| commonjs: true, | |
| es6: true, | |
| }, | |
| parserOptions: { | |
| ecmaVersion: 2020, | |
| sourceType: 'script', | |
| }, | |
| rules: { | |
| '@typescript-eslint/no-var-requires': 'off', | |
| 'no-undef': 'off', | |
| }, | |
| }, | |
| { | |
| files: ['**/*.{test,spec}.{ts,tsx,js,jsx}', 'tests/**/*.{ts,tsx,js,jsx}'], | |
| env: { | |
| jest: true, // Vitest runs Jest-style tests | |
| }, | |
| }, | |
| ], |
🤖 Prompt for AI Agents
In packages/sdk-alpha/.eslintrc.cjs around lines 54 to 71, add an override for
test files to define the Vitest globals like describe, it, and test to prevent
ESLint from flagging them as undefined. You can do this by adding a new override
entry targeting test files (e.g., '**/*.test.ts' or similar) and setting env to
include 'vitest/globals': true. If you prefer, install eslint-plugin-vitest and
configure the env accordingly. Also, ensure eslint-import-resolver-typescript
and eslint-import-resolver-node are added to devDependencies and configure
settings['import/resolver'] if import resolution issues arise.
|
|
||
| for (const f of files) { | ||
| if (!f.endsWith('.js')) continue; | ||
| const src = await readFile(join(dist.pathname, f), 'utf8'); |
There was a problem hiding this comment.
Fix cross‑platform path handling (use file URL, not pathname).
Using join(dist.pathname, f) can break on Windows and with URL-encoded paths. Read via the file URL directly.
- const src = await readFile(join(dist.pathname, f), 'utf8');
+ const src = await readFile(new URL(f, dist), 'utf8');📝 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.
| const src = await readFile(join(dist.pathname, f), 'utf8'); | |
| const src = await readFile(new URL(f, dist), 'utf8'); |
🤖 Prompt for AI Agents
In packages/sdk-alpha/scripts/report-exports.mjs at line 10, replace the use of
join(dist.pathname, f) with a method that uses the file URL directly to read the
file. Instead of constructing the path with dist.pathname, use the URL or
convert the URL to a path-safe format that works cross-platform, ensuring
compatibility with Windows and URL-encoded paths. This will fix the
cross-platform path handling issue.
| const direct = [...src.matchAll(/export\s+(?:const|function|class|let|var)\s+([A-Za-z0-9_$]+)/g)].map(m => m[1]); | ||
| const re = [...src.matchAll(/export\s*{([^}]+)}/g)] | ||
| .flatMap(m => m[1].split(',').map(s => s.trim())) | ||
| .map(s => s.split('\s+as\s+').pop()) | ||
| .filter(name => name && name !== 'default'); | ||
| report[f] = [...direct, ...re]; |
There was a problem hiding this comment.
Export parsing bug: string split uses a non-regex, aliasing and async functions mishandled.
s.split('\s+as\s+')never matches (string split isn’t regex), soexport { a as b }yields incorrect names.export async function foo()isn’t captured.- Potential duplicates aren’t deduped.
Apply a safer extraction that handles aliases, async functions, and dedupes results.
- const direct = [...src.matchAll(/export\s+(?:const|function|class|let|var)\s+([A-Za-z0-9_$]+)/g)].map(m => m[1]);
- const re = [...src.matchAll(/export\s*{([^}]+)}/g)]
- .flatMap(m => m[1].split(',').map(s => s.trim()))
- .map(s => s.split('\s+as\s+').pop())
- .filter(name => name && name !== 'default');
- report[f] = [...direct, ...re];
+ const directVars = [...src.matchAll(/export\s+(?:const|let|var)\s+([$\w]+)/g)].map(m => m[1]);
+ const directFns = [...src.matchAll(/export\s+(?:async\s+)?function\s+([$\w]+)/g)].map(m => m[1]);
+ const directClasses = [...src.matchAll(/export\s+class\s+([$\w]+)/g)].map(m => m[1]);
+ const named = [...src.matchAll(/export\s*{([^}]+)}/g)]
+ .flatMap(m => m[1].split(',').map(s => s.trim()).filter(Boolean))
+ .map(s => {
+ const m = s.match(/^([$\w]+)(?:\s+as\s+([$\w]+))?$/);
+ if (!m) return null;
+ const [, orig, alias] = m;
+ const name = alias || orig;
+ return name === 'default' ? null : name; // exclude final 'default' export
+ })
+ .filter(Boolean);
+ report[f] = Array.from(new Set([...directVars, ...directFns, ...directClasses, ...named]));Note: This still won’t account for export * as ns from or export * from (by design of the current approach).
📝 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.
| const direct = [...src.matchAll(/export\s+(?:const|function|class|let|var)\s+([A-Za-z0-9_$]+)/g)].map(m => m[1]); | |
| const re = [...src.matchAll(/export\s*{([^}]+)}/g)] | |
| .flatMap(m => m[1].split(',').map(s => s.trim())) | |
| .map(s => s.split('\s+as\s+').pop()) | |
| .filter(name => name && name !== 'default'); | |
| report[f] = [...direct, ...re]; | |
| const directVars = [...src.matchAll(/export\s+(?:const|let|var)\s+([$\w]+)/g)].map(m => m[1]); | |
| const directFns = [...src.matchAll(/export\s+(?:async\s+)?function\s+([$\w]+)/g)].map(m => m[1]); | |
| const directClasses = [...src.matchAll(/export\s+class\s+([$\w]+)/g)].map(m => m[1]); | |
| const named = [...src.matchAll(/export\s*{([^}]+)}/g)] | |
| .flatMap(m => m[1].split(',').map(s => s.trim()).filter(Boolean)) | |
| .map(s => { | |
| const m = s.match(/^([$\w]+)(?:\s+as\s+([$\w]+))?$/); | |
| if (!m) return null; | |
| const [, orig, alias] = m; | |
| const name = alias || orig; | |
| return name === 'default' ? null : name; // exclude final "default" | |
| }) | |
| .filter(Boolean); | |
| report[f] = Array.from(new Set([...directVars, ...directFns, ...directClasses, ...named])); |
🤖 Prompt for AI Agents
In packages/sdk-alpha/scripts/report-exports.mjs around lines 11 to 16, the
export parsing logic incorrectly uses string split with a regex pattern, fails
to capture async functions, and does not remove duplicate export names. Fix this
by replacing the string split with a regex-based extraction to correctly handle
aliasing in export statements, update the direct export regex to include async
functions, and add a step to deduplicate the combined export names before
assigning them to report[f].
| @@ -0,0 +1,20 @@ | |||
| import { describe, expect, it } from 'vitest'; | |||
|
|
|||
| import { extractMRZInfo, formatDateToYYMMDD } from '../src/processing/mrz.js'; | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Prefer importing from the package entry to test the public API (and verify ESM/TS config).
Testing via ../src/processing/mrz.js couples tests to internal structure. Import from the package entry to assert the public API surface and avoid brittle paths.
Proposed change:
-import { extractMRZInfo, formatDateToYYMMDD } from '../src/processing/mrz.js';
+// If the package entry re-exports these:
+import { extractMRZInfo, formatDateToYYMMDD } from '../src/index.js';
+// Alternatively (stronger guarantee you're testing the published API):
+// import { extractMRZInfo, formatDateToYYMMDD } from '@selfxyz/sdk-alpha';If you’re intentionally using ESM with TS and NodeNext/Bundler resolution, the .js extension in TS imports is correct. Please verify the package/module settings to avoid intermittent resolution issues:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking module type and exports for sdk-alpha..."
if fd -t f package.json packages/sdk-alpha >/dev/null 2>&1; then
echo "packages/sdk-alpha/package.json:"
cat packages/sdk-alpha/package.json | jq '{name, type, main, module, exports}'
fi
echo
echo "packages/sdk-alpha/tsconfig.json (module + moduleResolution):"
if [ -f packages/sdk-alpha/tsconfig.json ]; then
rg -n '"module"\s*:' packages/sdk-alpha/tsconfig.json -n -A1 || true
rg -n '"moduleResolution"\s*:' packages/sdk-alpha/tsconfig.json -n -A1 || true
fi
echo
echo "Confirm index re-exports:"
rg -n "export\s+\{[^}]*extractMRZInfo|formatDateToYYMMDD" packages/sdk-alpha/src/index.ts -n -A2 || trueLength of output: 513
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== packages/sdk-alpha/package.json summary ==="
jq '{name: .name, type: .type, main: .main, module: .module, exports: .exports}' packages/sdk-alpha/package.json || true
echo
echo "=== packages/sdk-alpha/tsconfig.json compilerOptions (module & moduleResolution) ==="
jq -r '.compilerOptions | {module, moduleResolution}' packages/sdk-alpha/tsconfig.json || true
echo
echo "=== Checking index re-exports in packages/sdk-alpha/src/index.ts ==="
grep -R "extractMRZInfo" packages/sdk-alpha/src/index.ts -n -A2 || echo "✖ extractMRZInfo not re-exported"
echo
grep -R "formatDateToYYMMDD" packages/sdk-alpha/src/index.ts -n -A2 || echo "✖ formatDateToYYMMDD not re-exported"Length of output: 1469
Use the package’s public API for MRZ helpers
We’ve confirmed that packages/sdk-alpha/src/index.ts re-exports both extractMRZInfo and formatDateToYYMMDD. To avoid coupling your tests to internal file paths—and to exercise the same entry point your consumers will use—please import from the package entry:
• File: packages/sdk-alpha/tests/mrz.test.ts
• Line: 3
- import { extractMRZInfo, formatDateToYYMMDD } from '../src/processing/mrz.js';
+ import { extractMRZInfo, formatDateToYYMMDD } from '../src/index.js';
//—or, for a truer public-API test:
// import { extractMRZInfo, formatDateToYYMMDD } from '@selfxyz/sdk-alpha';This ensures your tests verify the published API surface, and it sidesteps any ESM/TS path‐resolution quirks.
📝 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.
| import { extractMRZInfo, formatDateToYYMMDD } from '../src/processing/mrz.js'; | |
| import { extractMRZInfo, formatDateToYYMMDD } from '../src/index.js'; | |
| //—or, for a truer public-API test: | |
| // import { extractMRZInfo, formatDateToYYMMDD } from '@selfxyz/sdk-alpha'; |
🤖 Prompt for AI Agents
In packages/sdk-alpha/tests/mrz.test.ts at line 3, the import statement directly
references internal source files instead of using the package's public API. To
fix this, change the import to come from the package entry point (e.g.,
'sdk-alpha' or the appropriate package name) instead of
'../src/processing/mrz.js'. This will ensure the tests use the public API
surface and avoid coupling to internal file paths.
| it('extracts fields from TD3 MRZ', () => { | ||
| const sample = | ||
| 'P<UTOERIKSSON<<ANNA<MARIA<<<<<<<<<<<<<<<<<<<\nL898902C36UTO7408122F1204159ZE184226B<<<<<10'; | ||
| const info = extractMRZInfo(sample); | ||
| expect(info.passportNumber).toBe('L898902C3'); | ||
| expect(info.dateOfBirth).toBe('740812'); | ||
| expect(info.dateOfExpiry).toBe('120415'); | ||
| expect(info.documentType).toBe('P'); | ||
| expect(info.countryCode).toBe('UTO'); | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Broaden MRZ parsing tests: add invalid cases and optional field checks.
Good happy-path coverage for TD3. Add negative and edge cases to harden the parser and prevent regressions.
Consider adding:
- Invalid check digits (passport number, DOB, expiry) should be detected (throw or return validation flags).
- Line length validation (TD3 must be 2 lines × 44 chars). Test too-short/too-long lines.
- Non-TD3 input (TD1/TD2) behavior—either supported and parsed or explicitly rejected.
- Optional fields if exposed: surname, given names, nationality, sex, composite check digit.
Example additions (adapt as per API: throw vs. result.valid flags):
it('rejects MRZ with invalid check digits', () => {
const corrupted = 'P<UTOERIKSSON<<ANNA<MARIA<<<<<<<<<<<<<<<<<<<\nL898902C36UTO7408122F1204150ZE184226B<<<<<10'; // last line checksum digit altered
expect(() => extractMRZInfo(corrupted)).toThrow();
});
it('rejects MRZ with invalid TD3 line lengths', () => {
const badLen = 'P<UTOERIKSSON<<ANNA<MARIA<<<<<<<<<<<<<<<<<\nL898902C36UTO7408122F1204159ZE184226B<<<<<10'; // one char short on line 1
expect(() => extractMRZInfo(badLen)).toThrow();
});If the function returns richer data, assert them to lock behavior:
// Only if these fields are returned:
expect(info.sex).toBe('F');
expect(info.nationality).toBe('UTO');
// Optionally: expect(info.checksums).toEqual({ passportNumber: true, dateOfBirth: true, dateOfExpiry: true, composite: true });Minor readability nit: use a template string to make the two-line MRZ more readable:
-const sample = 'P<UTOERIKSSON<<ANNA<MARIA<<<<<<<<<<<<<<<<<<<\nL898902C36UTO7408122F1204159ZE184226B<<<<<10';
+const sample = `P<UTOERIKSSON<<ANNA<MARIA<<<<<<<<<<<<<<<<<<<
+L898902C36UTO7408122F1204159ZE184226B<<<<<10`;🏁 Script executed:
#!/bin/bash
# Locate the definition and usage of extractMRZInfo in the SDK
rg 'extractMRZInfo' -n .Length of output: 1259
#!/bin/bash
# Display the implementation of extractMRZInfo and related helpers
sed -n '1,500p' packages/sdk-alpha/src/processing/mrz.ts🏁 Script executed:
#!/bin/bash
# Inspect the implementation of extractMRZInfo for existing validations
sed -n '1,200p' packages/sdk-alpha/src/processing/mrz.ts
sed -n '201,400p' packages/sdk-alpha/src/processing/mrz.tsLength of output: 1971
Broaden and strengthen MRZ parsing: add validation and edge-case tests
The current extractMRZInfo helper only slices out five fields without validating MRZ structure, check digits, or line lengths. To harden the parser and prevent regressions, I recommend the following changes:
• In packages/sdk-alpha/src/processing/mrz.ts
- Enforce exact MRZ line counts and lengths (TD3 must be 2 lines × 44 chars).
- Compute and verify check digits for passport number, date of birth, date of expiry (and the composite checksum), or expose boolean flags in the return value.
- Decide on clear handling for unsupported formats (e.g. TD2) rather than accepting anything that matches the loose regex.
• In packages/sdk-alpha/tests/mrz.test.ts
- Add negative tests for invalid check digits and out-of-length lines, expecting either an
Erroror avalid: falseflag. - Test non-TD3/TD1 strings to confirm they’re rejected.
- Once the parser returns richer data (sex, nationality, optional fields), lock in those fields with assertions:
expect(info.sex).toBe('F'); expect(info.nationality).toBe('UTO'); expect(info.checksums).toEqual({ passportNumber: true, dateOfBirth: true, dateOfExpiry: true, composite: true });
- For readability, use a template string for the sample MRZ:
- const sample = 'P<UTOERIKSSON<<ANNA<MARIA<<<<<<<<<<<<<<<<<<<\nL898902C36UTO7408122F1204159ZE184226B<<<<<10'; + const sample = `P<UTOERIKSSON<<ANNA<MARIA<<<<<<<<<<<<<<<<<<< + L898902C36UTO7408122F1204159ZE184226B<<<<<10`;
These updates will catch malformed input early, lock in expected behavior, and guard against future regressions.
🤖 Prompt for AI Agents
In packages/sdk-alpha/src/processing/mrz.ts and tests/mrz.test.ts around lines 6
to 15, enhance the MRZ parser by enforcing strict validation of MRZ format
including exact line counts and lengths for TD3, verifying check digits for
passport number, birth date, expiry date, and composite checksum, and clearly
handling unsupported formats. Update the tests to include negative cases for
invalid check digits and incorrect line lengths expecting errors or a valid flag
set to false, add tests for non-TD3/TD1 inputs to confirm rejection, and expand
assertions to cover additional fields like sex, nationality, and checksum
results. Also, improve test readability by using template strings for MRZ
samples.
| it('formats ISO date to YYMMDD', () => { | ||
| expect(formatDateToYYMMDD('1974-08-12')).toBe('740812'); | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Harden date formatting tests against timezone and input variations.
The ISO string case is good; add cases to ensure no TZ bleed and correct 2000+ year handling.
Consider adding:
- Explicit UTC/Z offset forms to ensure invariance:
expect(formatDateToYYMMDD('1974-08-12T00:00:00Z')).toBe('740812');
expect(formatDateToYYMMDD('1974-08-12T23:59:59+00:00')).toBe('740812');- 2000+ years (ambiguity in YY):
expect(formatDateToYYMMDD('2003-01-05')).toBe('030105');- Non-delimited forms if supported (request verification):
// Only if accepted by API:
expect(formatDateToYYMMDD('19740812')).toBe('740812');Optionally, add a negative test for invalid dates:
expect(() => formatDateToYYMMDD('not-a-date')).toThrow();If this utility accepts Date objects, also assert:
expect(formatDateToYYMMDD(new Date(Date.UTC(1974, 7, 12)))).toBe('740812');Harden date‐formatting tests for time zones, century rollover, and invalid inputs
The existing ISO-date case is great; let’s expand coverage:
• Add explicit UTC/Z-offset variants to guard against TZ bleed:
expect(formatDateToYYMMDD('1974-08-12T00:00:00Z')).toBe('740812');
expect(formatDateToYYMMDD('1974-08-12T23:59:59+00:00')).toBe('740812');• Verify century rollover into 2000-plus years:
expect(formatDateToYYMMDD('2003-01-05')).toBe('030105');• Add a negative test for invalid strings:
expect(() => formatDateToYYMMDD('not-a-date')).toThrow('Invalid date');Current implementation only accepts date-strings and throws on invalid dates—it doesn’t yet parse non-delimited inputs (e.g. '19740812') or Date objects. If you plan to support those in the future, add them and their corresponding tests; otherwise you can drop those cases for now.
🤖 Prompt for AI Agents
In packages/sdk-alpha/tests/mrz.test.ts around lines 17 to 19, the
date-formatting test only covers a basic ISO date string. To harden the tests,
add cases with explicit UTC and Z-offset time zone variants to ensure no time
zone bleed affects the output. Also, add a test for century rollover by
including a date from the 2000s. Finally, include a negative test case that
verifies the function throws an error when given an invalid date string. This
will improve coverage and robustness of the date formatting function.
Summary
@selfxyz/sdk-alphapackage and expose MRZ helpersTesting
yarn lintyarn buildyarn workspace @selfxyz/contracts build(fails: Invalid account for network)yarn typesyarn workspace @selfxyz/common testyarn workspace @selfxyz/circuits test(fails: unsupported signature algorithm)yarn workspace @selfxyz/mobile-app testhttps://chatgpt.com/codex/tasks/task_b_6895226a0ff8832db4d496e81dcff690
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores