feat: use SelfClient for MRZ parsing #930
Conversation
WalkthroughAdds a SelfClientProvider and switches PassportCamera to call SelfClient.extractMRZInfo; introduces provider and unit tests; adds license-header checking/fixing scripts and lint script updates; expands mobile CI workflow and caching and updates macOS runner to macos-latest. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App
participant SelfClientProvider
participant AuthProvider
participant PassportProvider
participant OtherProviders
User->>App: Launch
App->>SelfClientProvider: Mount (create adapters: scanner, http, ws, crypto)
SelfClientProvider->>AuthProvider: Render children
AuthProvider->>PassportProvider: Render
PassportProvider->>OtherProviders: Render downstream providers/navigation
sequenceDiagram
autonumber
participant PassportCamera
participant SelfClient as SelfClient (from Provider)
participant onPassportRead as Callback (prop)
PassportCamera->>PassportCamera: receive MRZ (string or object)
alt MRZ is string
PassportCamera->>SelfClient: extractMRZInfo(mrzString)
SelfClient-->>PassportCamera: parsedMRZ
PassportCamera-->>onPassportRead: (null, parsedMRZ)
else MRZ is object
PassportCamera-->>onPassportRead: (null, mappedObject)
end
opt Web stub
Note right of PassportCamera: on mount, schedules error after 100ms
PassportCamera-->>onPassportRead: (Error, undefined)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
✨ 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/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
| jobs: | ||
| lint: | ||
| runs-on: macos-14 | ||
| runs-on: macos-latest |
There was a problem hiding this comment.
this fixes the failing 16.4 error. missed it from earlier
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/mobile-ci.yml (1)
235-238: Don’t delete Podfile.lock; it undermines reproducible iOS buildsRemoving Podfile.lock during cleanup forces a fresh dependency resolution, leading to non-deterministic builds and flaky CI. Keep Podfile.lock to ensure reproducibility.
Apply this diff:
- echo "Cleaning empty/corrupted Pods directory..." - rm -rf Pods Podfile.lock + echo "Cleaning empty/corrupted Pods directory (preserving Podfile.lock for reproducible installs)..." + rm -rf Podsapp/src/components/native/PassportCamera.tsx (1)
93-99: Handle MRZ parsing failures to avoid runtime crashesselfClient.extractMRZInfo may throw on malformed MRZ input. Catch and surface the error via onPassportRead.
Apply this diff:
if (typeof event.nativeEvent.data === 'string') { - onPassportRead(null, selfClient.extractMRZInfo(event.nativeEvent.data)); + try { + const parsed = selfClient.extractMRZInfo(event.nativeEvent.data); + onPassportRead(null, parsed); + } catch (err) { + const e = err instanceof Error ? err : new Error(String(err)); + onPassportRead(e); + } } else {
🧹 Nitpick comments (2)
.github/workflows/mobile-ci.yml (2)
130-136: Avoid brittle manual xcode-select; rely on setup-xcode to manage DEVELOPER_DIRManually switching to /Applications/Xcode_${{ env.XCODE_VERSION }}.app is fragile and can break if the image path changes. setup-xcode sets DEVELOPER_DIR for the chosen version; prefer relying on it and keep only diagnostics.
Apply this diff to remove the manual switch:
echo "🔧 Configuring Xcode path to fix iOS SDK issues..." - # Fix for macOS 15 runner iOS SDK issues - # See: https://github.com/actions/runner-images/issues/12758 - sudo xcode-select --switch /Applications/Xcode_${{ env.XCODE_VERSION }}.app - echo "✅ Xcode path configured" + # Using setup-xcode to set DEVELOPER_DIR; no manual xcode-select required + echo "✅ Xcode path configured by setup-xcode"
22-22: Ensure Xcode 16.4 is always available on your macOS runnersmacos-latest is currently rolling from macos-14 → macos-15 (Aug 4–Sep 1, 2025). On macos-14 images Xcode 16.4 may be absent, causing your
actions/setup-xcodestep to fail.• Files & lines to update:
– .github/workflows/mobile-ci.yml @ lines 22, 65, 105• Options to guarantee Xcode 16.4:
– Pin the runner explicitly:
yaml runs-on: macos-15
– Or verify before switching:
yaml - name: Verify Xcode 16.4 run: | xcodebuild -version ls /Applications | grep Xcode_16.4.app
• Leverageactions/setup-xcodeto select the version and remove any manualxcode-selectinvocations.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
.github/workflows/mobile-ci.yml(3 hunks)app/App.tsx(2 hunks)app/src/components/native/PassportCamera.tsx(3 hunks)app/src/components/native/PassportCamera.web.tsx(3 hunks)app/src/providers/selfClientProvider.tsx(1 hunks)app/tests/src/components/PassportCamera.test.tsx(1 hunks)app/tests/src/providers/selfClientProvider.test.tsx(1 hunks)app/tests/src/utils/proving/validateDocument.test.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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:
app/tests/src/components/PassportCamera.test.tsxapp/tests/src/utils/proving/validateDocument.test.tsapp/tests/src/providers/selfClientProvider.test.tsx
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/providers/selfClientProvider.tsxapp/src/components/native/PassportCamera.tsxapp/src/components/native/PassportCamera.web.tsx
🧠 Learnings (1)
📚 Learning: 2025-07-14T09:03:08.292Z
Learnt from: aaronmgdr
PR: selfxyz/self#763
File: app/.github/workflows/test-coverage.yml:0-0
Timestamp: 2025-07-14T09:03:08.292Z
Learning: Node.js 22 is supported by actions/setup-nodev4 and has been in Active LTS since October 2024. It works on Linux, macOS, and Windows runners (Windows issues were resolved after version 22.8.0).
Applied to files:
.github/workflows/mobile-ci.yml
🧬 Code Graph Analysis (5)
app/App.tsx (1)
app/src/providers/selfClientProvider.tsx (1)
SelfClientProvider(18-70)
app/tests/src/utils/proving/validateDocument.test.ts (2)
packages/mobile-sdk-alpha/src/browser.ts (1)
createSelfClient(46-46)packages/mobile-sdk-alpha/src/index.ts (1)
createSelfClient(75-75)
app/tests/src/providers/selfClientProvider.test.tsx (1)
app/src/providers/selfClientProvider.tsx (1)
SelfClientProvider(18-70)
app/src/components/native/PassportCamera.tsx (4)
packages/mobile-sdk-alpha/src/types/public.ts (1)
SelfClient(119-135)app/src/components/native/PassportCamera.web.tsx (2)
PassportCamera(17-76)PassportCameraProps(9-15)app/android/app/src/main/java/com/proofofpassportapp/ui/PassportOCRViewManager.kt (1)
onPassportRead(131-137)app/android/app/src/main/java/com/proofofpassportapp/ui/CameraMLKitFragment.kt (2)
onPassportRead(404-407)onPassportRead(405-405)
app/src/components/native/PassportCamera.web.tsx (2)
app/src/components/native/PassportCamera.tsx (2)
PassportCameraProps(46-52)PassportCamera(54-158)packages/mobile-sdk-alpha/src/types/public.ts (1)
SelfClient(119-135)
⏰ 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). (2)
- GitHub Check: e2e-ios
- GitHub Check: analyze-android
🔇 Additional comments (5)
app/App.tsx (1)
29-39: Provider placement looks correctWrapping the app with SelfClientProvider above Auth/Passport/DB providers ensures hook availability across the tree.
app/src/components/native/PassportCamera.tsx (1)
46-51: Type alignment with SelfClient is correctUpdating mrzData to ReturnType<SelfClient['extractMRZInfo']> keeps the prop in sync with SDK types.
app/tests/src/utils/proving/validateDocument.test.ts (3)
54-81: Solid test client scaffolding; adapters are sufficiently stubbed for MRZ parsing.The minimal adapters are adequate for exercising
extractMRZInfowithout leaking network/crypto side effects. No blockers here.
175-180: Good positive-path MRZ assertion.Verifies both field extraction and validation, aligned with the SDK’s expected behavior for the ICAO sample.
182-185: Good negative-path coverage.Ensures malformed input surfaces an error from the SDK parser.
| * Ensures there's a newline after license headers | ||
| */ | ||
|
|
||
| // SPDX-License-Identifier: BUSL-1.1; Copyright (c) 2025 Social Connect Labs, Inc.; Licensed under BUSL-1.1 (see LICENSE); Apache-2.0 from 2029-06-11 |
There was a problem hiding this comment.
fixes issue with header license eslint fix not working
transphorm
changed the title
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
app/package.json (1)
38-41: Minor duplication: lint:headers:fix runs twice in “nice”.nice runs lint:headers:fix and then lint:fix, which itself runs lint:headers:fix. You can drop the first call to speed up CI/local runs.
- "nice": "yarn imports:fix && yarn lint:headers:fix && yarn lint:fix && yarn fmt:fix", + "nice": "yarn imports:fix && yarn lint:fix && yarn fmt:fix",Also applies to: 47-47
app/tests/src/utils/proving/validateDocument.test.ts (1)
55-83: Strengthen the WsConn mock to match the callback signaturesThe ws.connect mock returns jest.fn placeholders for onMessage/onError/onClose without accepting callbacks. If future tests rely on these registrations, they’ll silently pass without actually wiring handlers. Prefer no-op implementations that accept and discard callbacks so signature drift is caught.
Apply this diff:
ws: { connect: jest.fn(() => ({ send: jest.fn(), close: jest.fn(), - onMessage: jest.fn(), - onError: jest.fn(), - onClose: jest.fn(), + onMessage: jest.fn((_cb: (data: unknown) => void) => {}), + onError: jest.fn((_cb: (err: unknown) => void) => {}), + onClose: jest.fn((_cb: () => void) => {}), })), },app/src/providers/selfClientProvider.tsx (1)
29-47: Harden WebSocket adapter: set binaryType and normalize error payloadsTwo robustness tweaks will prevent subtle runtime issues:
- Without setting binaryType, browsers may deliver Blob payloads; downstream code often expects string or ArrayBuffer.
- The error event listener forwards a generic Event. Converting to an Error improves downstream error handling.
Apply this diff:
connect: (url: string): WsConn => { const socket = new WebSocket(url); + // Prefer ArrayBuffer for binary frames; avoids Blob handling downstream + try { + socket.binaryType = 'arraybuffer'; + } catch {} return { send: (data: string | ArrayBufferView | ArrayBuffer) => socket.send(data), close: () => socket.close(), onMessage: cb => { socket.addEventListener('message', ev => cb((ev as MessageEvent).data), ); }, onError: cb => { - socket.addEventListener('error', e => cb(e)); + socket.addEventListener('error', ev => { + const anyEv = ev as any; + const err = + anyEv?.error instanceof Error + ? anyEv.error + : new Error('WebSocket error'); + cb(err as unknown as Error); + }); }, onClose: cb => { socket.addEventListener('close', () => cb()); }, }; },app/tests/src/providers/selfClientProvider.test.tsx (1)
21-56: Good isolation and cleanup; consider exercising adapters via the clientThe test safely stubs and restores globals. Note that it currently verifies environment primitives (crypto/fetch/WebSocket) rather than the SDK using the provided adapters. If feasible, add a lightweight assertion that a client operation routes through these adapters (e.g., a method that invokes network/crypto) to catch integration regressions.
app/src/components/native/PassportCamera.web.tsx (1)
22-29: Guard MRZ parsing with try/catch to avoid unhandled exceptions in UI callbacksIf extractMRZInfo throws, the error will bubble out of the React event/callback and potentially crash the component. Route parsing errors to onPassportRead(error) so consumers can handle failure gracefully.
Apply this diff:
const _onPassportRead = useCallback( (mrz: string) => { if (!isMounted) { return; } - onPassportRead(null, selfClient.extractMRZInfo(mrz)); + try { + const info = selfClient.extractMRZInfo(mrz); + onPassportRead(null, info); + } catch (err) { + const e = err instanceof Error ? err : new Error(String(err)); + onPassportRead(e); + } }, [onPassportRead, isMounted, selfClient], );
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
app/.eslintrc.cjs(1 hunks)app/package.json(1 hunks)app/scripts/check-duplicate-headers.cjs(1 hunks)app/scripts/check-license-headers.mjs(1 hunks)app/src/components/native/PassportCamera.tsx(4 hunks)app/src/components/native/PassportCamera.web.tsx(3 hunks)app/src/providers/selfClientProvider.tsx(1 hunks)app/tests/src/components/PassportCamera.test.tsx(1 hunks)app/tests/src/providers/selfClientProvider.test.tsx(1 hunks)app/tests/src/utils/proving/validateDocument.test.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/components/native/PassportCamera.tsx
- app/tests/src/components/PassportCamera.test.tsx
🧰 Additional context used
📓 Path-based instructions (2)
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/providers/selfClientProvider.tsxapp/src/components/native/PassportCamera.web.tsx
**/*.{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:
app/tests/src/providers/selfClientProvider.test.tsxapp/tests/src/utils/proving/validateDocument.test.ts
🧬 Code Graph Analysis (4)
app/tests/src/providers/selfClientProvider.test.tsx (1)
app/src/providers/selfClientProvider.tsx (1)
SelfClientProvider(19-81)
app/scripts/check-license-headers.mjs (2)
app/scripts/check-duplicate-headers.cjs (5)
path(5-5)files(40-42)fs(4-4)content(12-12)lines(13-13)app/scripts/analyze-tree-shaking.cjs (2)
findFiles(278-291)filePath(43-43)
app/src/components/native/PassportCamera.web.tsx (2)
app/src/components/native/PassportCamera.tsx (2)
PassportCameraProps(46-52)PassportCamera(54-158)packages/mobile-sdk-alpha/src/types/public.ts (1)
SelfClient(119-135)
app/tests/src/utils/proving/validateDocument.test.ts (2)
packages/mobile-sdk-alpha/src/browser.ts (1)
createSelfClient(46-46)packages/mobile-sdk-alpha/src/index.ts (1)
createSelfClient(75-75)
⏰ 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). (2)
- GitHub Check: e2e-ios
- GitHub Check: build
🔇 Additional comments (7)
app/.eslintrc.cjs (1)
96-104: LGTM: BOF allowance aligns with header-newline policy.Allowing maxBOF: 1 complements the dedicated newline-after-header script and avoids spurious ESLint violations after the license line.
Also applies to: 106-116
app/tests/src/utils/proving/validateDocument.test.ts (2)
84-89: Good MRZ fixtures; positive and negative paths coveredThe ICAO sample (validMrz) and malformed case (invalidMrz) are appropriate for exercising parse success and failure paths.
171-186: Verify no strayextractMRZInfoimports or calls remainOur initial grep found no direct imports of
extractMRZInfofrom@selfxyz/mobile-sdk-alpha, but the bare-call search was inconclusive. To complete the migration and avoid mixed usage patterns:• Manually search all
.ts/.tsx(and any.jsif you use JS) for
import { extractMRZInfo } from '@selfxyz/mobile-sdk-alpha'- bare calls like
extractMRZInfo(...)without aclient.receiver
• Ensure every MRZ parse is invoked viaclient.extractMRZInfoonlyThis will safeguard against hidden direct usages and keep the codebase consistent.
app/src/providers/selfClientProvider.tsx (2)
51-66: WebCrypto hashing adapter is resilient to RN/web environmentsThe subtle.digest availability guard and algorithm mapping are sensible. Given upstream callers may vary the casing/hyphenation, consider normalizing more broadly if you start accepting other identifiers, but as-is this is fine.
76-81: Provider wiring looks correct and minimalConfig/adapters memoization and passing through SDKSelfClientProvider are clean. No concerns.
app/tests/src/providers/selfClientProvider.test.tsx (1)
10-19: Memoization test is effective and conciseThe instance reference check across rerenders validates stable client provisioning.
app/src/components/native/PassportCamera.web.tsx (1)
39-51: Web stub behavior is explicit and predictableThe delayed error signaling makes the “not implemented” state obvious to callers. Dependencies are correct and avoid stale closures.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.github/workflows/mobile-ci.yml (3)
247-270: Use correct workspace path relative to working-directory in Verify iOS Workspace.This step runs in ./app/ios, but sets WORKSPACE_PATH="app/ios/...". That results in app/ios/app/ios/... and causes false “workspace not found” and scheme checks to fail.
Apply this diff:
- WORKSPACE_PATH="app/ios/${{ env.IOS_PROJECT_NAME }}.xcworkspace" + WORKSPACE_PATH="${{ env.IOS_PROJECT_NAME }}.xcworkspace" if [ ! -d "$WORKSPACE_PATH" ]; then echo "❌ Workspace not found at: $WORKSPACE_PATH" echo "Available workspaces:" - find app/ios -name "*.xcworkspace" -type d + find . -name "*.xcworkspace" -type d exit 1 fi @@ - AVAILABLE_SCHEMES=$(xcodebuild -list -workspace "$WORKSPACE_PATH" 2>/dev/null | grep -A 200 "Schemes:" | grep -v "Schemes:" | xargs) + AVAILABLE_SCHEMES=$(xcodebuild -list -workspace "$WORKSPACE_PATH" 2>/dev/null | grep -A 200 "Schemes:" | grep -v "Schemes:" | xargs) echo "Available schemes (first 20): $(echo $AVAILABLE_SCHEMES | cut -d' ' -f1-20)..." - if [[ ! "$AVAILABLE_SCHEMES" =~ ${{ env.IOS_PROJECT_SCHEME }} ]]; then + if [[ ! "$AVAILABLE_SCHEMES" =~ ${{ env.IOS_PROJECT_SCHEME }} ]]; then echo "❌ Scheme '${{ env.IOS_PROJECT_SCHEME }}' not found" echo "Full scheme list:" - xcodebuild -list -workspace "$WORKSPACE_PATH" 2>/dev/null | grep -A 200 "Schemes:" | grep -v "Schemes:" | head -50 + xcodebuild -list -workspace "$WORKSPACE_PATH" 2>/dev/null | grep -A 200 "Schemes:" | grep -v "Schemes:" | head -50 exit 1 fi
49-58: Align Node modules cache key with the app’s lockfile.The linter caches depend on yarn.lock at repo root, but installs run under ./app. If the real lockfile is app/yarn.lock, your cache key won’t invalidate on dependency changes.
- name: Cache Node Modules uses: actions/cache@v4 with: path: | .yarn/cache node_modules app/node_modules - key: ${{ runner.os }}-node-${{ env.NODE_VERSION_SANITIZED }}-yarn-${{ hashFiles('yarn.lock') }} + key: ${{ runner.os }}-node-${{ env.NODE_VERSION_SANITIZED }}-yarn-${{ hashFiles('app/yarn.lock') }} restore-keys: | ${{ runner.os }}-node-${{ env.NODE_VERSION_SANITIZED }}-yarn-
92-101: Use app/yarn.lock for test job cache key as well.Same concern as lint job: ensure cache invalidates when app dependencies change.
- name: Cache Node Modules uses: actions/cache@v4 with: path: | .yarn/cache node_modules app/node_modules - key: ${{ runner.os }}-node-${{ env.NODE_VERSION_SANITIZED }}-yarn-${{ hashFiles('yarn.lock') }} + key: ${{ runner.os }}-node-${{ env.NODE_VERSION_SANITIZED }}-yarn-${{ hashFiles('app/yarn.lock') }} restore-keys: | ${{ runner.os }}-node-${{ env.NODE_VERSION_SANITIZED }}-yarn-
♻️ Duplicate comments (2)
app/scripts/check-license-headers.mjs (2)
50-67: Header check misses files with a shebang and breaks on CRLF; preserves neither index nor EOL.Current logic only matches when the license is the very first line and splits on LF. Files with a shebang or CRLF endings will be false negatives; fixes will also clobber EOL style.
Apply this robust, cross-platform fix:
function checkLicenseHeader(filePath) { - const content = fs.readFileSync(filePath, 'utf8'); - const lines = content.split('\n'); - - // Check if first line is license header - if (lines[0] === LICENSE_HEADER) { - // Check if second line is empty (has newline after license header) - if (lines[1] !== '') { - return { - file: filePath, - issue: 'Missing newline after license header', - fixed: false, - }; - } - } - - return null; + const content = fs.readFileSync(filePath, 'utf8'); + // Preserve original EOLs and compare robustly + const eol = content.includes('\r\n') ? '\r\n' : '\n'; + const lines = content.split(eol); + + // License should follow an optional shebang + let idx = 0; + if (lines[0]?.startsWith('#!')) idx = 1; + + const normalize = s => (typeof s === 'string' ? s.replace(/\r$/, '') : s); + if (normalize(lines[idx]) === LICENSE_HEADER) { + if (normalize(lines[idx + 1]) !== '') { + return { + file: filePath, + issue: 'Missing newline after license header', + fixed: false, + }; + } + } + return null; }To quickly surface affected files in the repo (missing blank line after SPDX when a shebang is present), run:
#!/bin/bash set -euo pipefail # Scan code files, skipping heavy/build dirs fd -e ts -e tsx -e js -e jsx -e mjs -e cjs -E node_modules -E dist -E build -E coverage \ | while IFS= read -r f; do # Only consider files with a shebang followed by SPDX on the next line awk 'NR==1{sheb=($0 ~ /^#!/)} NR==2{spdx=($0 ~ /^\/\/ SPDX-License-Identifier:/); empty2=($0 ~ /^$/)} NR==3{empty3=($0 ~ /^$/)} END{ if (sheb && spdx && !empty3) print FILENAME }' "$f" done
69-82: Fixer will corrupt line endings and fails on shebang-indexed headers.It only inserts after line 0 and always rewrites with LF. This both misses valid cases and changes file EOLs.
Apply this fix to respect shebang placement and preserve EOL style:
function fixLicenseHeader(filePath) { - const content = fs.readFileSync(filePath, 'utf8'); - const lines = content.split('\n'); - - if (lines[0] === LICENSE_HEADER && lines[1] !== '') { - // Insert empty line after license header - lines.splice(1, 0, ''); - const fixedContent = lines.join('\n'); - fs.writeFileSync(filePath, fixedContent, 'utf8'); - return true; - } - - return false; + const content = fs.readFileSync(filePath, 'utf8'); + const eol = content.includes('\r\n') ? '\r\n' : '\n'; + const lines = content.split(eol); + + let idx = 0; + if (lines[0]?.startsWith('#!')) idx = 1; + const normalize = s => (typeof s === 'string' ? s.replace(/\r$/, '') : s); + + if (normalize(lines[idx]) === LICENSE_HEADER && normalize(lines[idx + 1]) !== '') { + // Insert a blank line immediately after the license header, preserving EOL + lines.splice(idx + 1, 0, ''); + const fixedContent = lines.join(eol); + fs.writeFileSync(filePath, fixedContent, 'utf8'); + return true; + } + return false; }
🧹 Nitpick comments (1)
.github/workflows/mobile-ci.yml (1)
160-167: Add Yarn cache to build job for network resilience and speed.Build job only caches app/node_modules; missing .yarn/cache leads to frequent network pulls and flakiness. Lint/test jobs already cache .yarn/cache.
- - name: Cache Node modules + - name: Cache Node modules uses: actions/cache@v4 with: - path: app/node_modules + path: | + .yarn/cache + app/node_modules key: ${{ runner.os }}-node-${{ env.NODE_VERSION_SANITIZED }}-${{ hashFiles('app/yarn.lock') }} restore-keys: | ${{ runner.os }}-node-${{ env.NODE_VERSION_SANITIZED }}-
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
app/Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
.github/workflows/mobile-ci.yml(8 hunks)app/package.json(1 hunks)app/scripts/check-license-headers.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/package.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-14T09:03:08.292Z
Learnt from: aaronmgdr
PR: selfxyz/self#763
File: app/.github/workflows/test-coverage.yml:0-0
Timestamp: 2025-07-14T09:03:08.292Z
Learning: Node.js 22 is supported by actions/setup-nodev4 and has been in Active LTS since October 2024. It works on Linux, macOS, and Windows runners (Windows issues were resolved after version 22.8.0).
Applied to files:
.github/workflows/mobile-ci.yml
🧬 Code Graph Analysis (1)
app/scripts/check-license-headers.mjs (2)
app/scripts/check-duplicate-headers.cjs (5)
path(5-5)files(40-42)fs(4-4)content(12-12)lines(13-13)app/scripts/analyze-tree-shaking.cjs (2)
findFiles(278-291)filePath(43-43)
⏰ 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). (2)
- GitHub Check: e2e-ios
- GitHub Check: analyze-android
🔇 Additional comments (3)
.github/workflows/mobile-ci.yml (1)
28-28: Confirm Xcode 16.4 availability on macos-latest or pin the runner.You hard-pin Xcode to 16.4 and run on macos-latest. If the image rotation ever drops 16.4, builds will break. Consider pinning to macos-15 or adding a fallback strategy.
If you want to keep macos-latest, a pragmatic safeguard is to allow an alternate 16.x:
- - name: Set up Xcode + - name: Set up Xcode uses: maxim-lobanov/setup-xcode@v1 with: - xcode-version: ${{ env.XCODE_VERSION }} + xcode-version: ^${{ env.XCODE_VERSION }} # any compatible 16.x if 16.4 is unavailableAlternatively, pin the runner:
- runs-on: macos-latest + runs-on: macos-15Also applies to: 71-71, 111-111
app/scripts/check-license-headers.mjs (2)
3-9: SPDX header placement looks correct after the shebang.This should satisfy eslint-plugin-header’s “line” mode.
20-23: Good catch including .mjs and .cjs.This ensures the checker covers Node ESM/CommonJS scripts as well.
| uses: actions/cache@v4 | ||
| with: | ||
| path: | | ||
| app/ios/build | ||
| ~/Library/Developer/Xcode/DerivedData | ||
| ~/Library/Caches/com.apple.dt.Xcode | ||
| key: ${{ runner.os }}-xcode-${{ hashFiles('app/ios/Podfile.lock') }}-${{ hashFiles('app/ios/${{ env.IOS_PROJECT_NAME }}.xcworkspace/contents.xcworkspacedata') }} | ||
| restore-keys: | |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Cache key won’t include workspace hash due to unevaluated expression inside hashFiles().
hashFiles() won’t expand ${{ env.IOS_PROJECT_NAME }} when it’s inside the quoted path literal. The call will likely return an empty hash, reducing cache uniqueness.
Use a glob that doesn’t require expression interpolation:
- key: ${{ runner.os }}-xcode-${{ hashFiles('app/ios/Podfile.lock') }}-${{ hashFiles('app/ios/${{ env.IOS_PROJECT_NAME }}.xcworkspace/contents.xcworkspacedata') }}
+ key: ${{ runner.os }}-xcode-${{ hashFiles('app/ios/Podfile.lock') }}-${{ hashFiles('app/ios/*.xcworkspace/contents.xcworkspacedata') }}📝 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.
| uses: actions/cache@v4 | |
| with: | |
| path: | | |
| app/ios/build | |
| ~/Library/Developer/Xcode/DerivedData | |
| ~/Library/Caches/com.apple.dt.Xcode | |
| key: ${{ runner.os }}-xcode-${{ hashFiles('app/ios/Podfile.lock') }}-${{ hashFiles('app/ios/${{ env.IOS_PROJECT_NAME }}.xcworkspace/contents.xcworkspacedata') }} | |
| restore-keys: | | |
| uses: actions/cache@v4 | |
| with: | |
| path: | | |
| app/ios/build | |
| ~/Library/Developer/Xcode/DerivedData | |
| ~/Library/Caches/com.apple.dt.Xcode | |
| key: ${{ runner.os }}-xcode-${{ hashFiles('app/ios/Podfile.lock') }}-${{ hashFiles('app/ios/*.xcworkspace/contents.xcworkspacedata') }} | |
| restore-keys: | |
🤖 Prompt for AI Agents
In .github/workflows/mobile-ci.yml around lines 181-188, the hashFiles() call
embeds the env expression inside a quoted path so ${{ env.IOS_PROJECT_NAME }}
won’t be expanded, making the workspace hash empty; fix by using a glob that
doesn’t require interpolation (e.g.
hashFiles('app/ios/**/contents.xcworkspacedata')) and include the
IOS_PROJECT_NAME separately in the cache key (e.g. ...-${{ env.IOS_PROJECT_NAME
}}), or alternatively compute the workspace path before using hashFiles so the
path string passed to hashFiles contains no unevaluated expressions.
| run: | | ||
| echo "Installing iOS dependencies..." | ||
| # Clean Pods directory if it's corrupted or empty | ||
| if [ ! -d "Pods" ] || [ -z "$(ls -A Pods 2>/dev/null)" ]; then | ||
| echo "Cleaning empty/corrupted Pods directory..." | ||
| rm -rf Pods Podfile.lock | ||
| fi | ||
| # Install pods | ||
| bundle exec pod install --silent || { echo "❌ Pod install failed"; exit 1; } | ||
| (cd app/ios && pod install --silent) || { echo "❌ Pod install failed"; exit 1; } | ||
| echo "✅ Pods installed successfully" | ||
| # Verify installation | ||
| if [ ! -d "Pods" ] || [ -z "$(ls -A Pods 2>/dev/null)" ]; then | ||
| echo "❌ Pods directory is still empty after installation" | ||
| exit 1 | ||
| fi | ||
| echo "Pods directory contents:" | ||
| ls -la Pods/ | head -10 | ||
| # Verify key files exist | ||
| if [ ! -f "Pods/Target Support Files/Pods-Self/Pods-Self.debug.xcconfig" ]; then | ||
| echo "❌ Key CocoaPods configuration file missing" | ||
| exit 1 | ||
| fi | ||
| echo "✅ iOS dependencies installed successfully" | ||
| working-directory: ./app/ios | ||
| - name: Verify iOS Workspace |
There was a problem hiding this comment.
Fix double-directory traversal in Pods install (cd into app/ios twice).
The step’s working-directory is already ./app/ios. The subshell cd app/ios causes path resolution to ./app/ios/app/ios, breaking pod install.
Apply this diff:
- - name: Install iOS Dependencies
+ - name: Install iOS Dependencies
run: |
echo "Installing iOS dependencies..."
- (cd app/ios && pod install --silent) || { echo "❌ Pod install failed"; exit 1; }
+ pod install --silent || { echo "❌ Pod install failed"; exit 1; }
echo "✅ Pods installed successfully"
working-directory: ./app/ios📝 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.
| run: | | |
| echo "Installing iOS dependencies..." | |
| # Clean Pods directory if it's corrupted or empty | |
| if [ ! -d "Pods" ] || [ -z "$(ls -A Pods 2>/dev/null)" ]; then | |
| echo "Cleaning empty/corrupted Pods directory..." | |
| rm -rf Pods Podfile.lock | |
| fi | |
| # Install pods | |
| bundle exec pod install --silent || { echo "❌ Pod install failed"; exit 1; } | |
| (cd app/ios && pod install --silent) || { echo "❌ Pod install failed"; exit 1; } | |
| echo "✅ Pods installed successfully" | |
| # Verify installation | |
| if [ ! -d "Pods" ] || [ -z "$(ls -A Pods 2>/dev/null)" ]; then | |
| echo "❌ Pods directory is still empty after installation" | |
| exit 1 | |
| fi | |
| echo "Pods directory contents:" | |
| ls -la Pods/ | head -10 | |
| # Verify key files exist | |
| if [ ! -f "Pods/Target Support Files/Pods-Self/Pods-Self.debug.xcconfig" ]; then | |
| echo "❌ Key CocoaPods configuration file missing" | |
| exit 1 | |
| fi | |
| echo "✅ iOS dependencies installed successfully" | |
| working-directory: ./app/ios | |
| - name: Verify iOS Workspace | |
| - name: Install iOS Dependencies | |
| run: | | |
| echo "Installing iOS dependencies..." | |
| pod install --silent || { echo "❌ Pod install failed"; exit 1; } | |
| echo "✅ Pods installed successfully" | |
| working-directory: ./app/ios |
🤖 Prompt for AI Agents
.github/workflows/mobile-ci.yml around lines 238 to 243: the run step currently
cds into app/ios even though the job already sets working-directory: ./app/ios,
causing a double traversal to ./app/ios/app/ios and breaking pod install; remove
the subshell cd and run pod install --silent directly (e.g., run: echo
"Installing iOS dependencies..." ; pod install --silent || { echo "❌ Pod install
failed"; exit 1; } ; echo "✅ Pods installed successfully") so the command
executes in the declared working-directory.
Summary
Testing
yarn install(fails: network errors)yarn workspace @selfxyz/mobile-app nice(fails: Couldn't find the node_modules state file)yarn workspace @selfxyz/mobile-app lint(fails: Couldn't find the node_modules state file)yarn workspace @selfxyz/mobile-app types(fails: Couldn't find the node_modules state file)yarn workspace @selfxyz/mobile-app test(fails: Couldn't find the node_modules state file)yarn workspace @selfxyz/mobile-app build(fails: Couldn't find the node_modules state file)https://chatgpt.com/codex/tasks/task_b_68a531b0f6d4832d8e8f99633c7c08a5
Summary by CodeRabbit