Add / update ci retry logic #1332
Conversation
WalkthroughThis pull request systematically adds retry logic via the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/actions/yarn-install/action.yml (1)
14-27: Clarify yarn caching strategy.Line 26 specifies
cache: "yarn"viasetup-node, while workflows separately invoke./.github/actions/cache-yarn(e.g., mobile-ci.yml line 64). This dual-caching approach may be redundant or conflicting. Either remove the built-in cache here, or document why both are needed.As per coding guidelines, workflows should use the shared
.github/actions/cache-yarncomposite action consistently. Consider removingcache: "yarn"from setup-node to avoid redundancy.- name: Setup Node.js uses: actions/setup-node@v4 with: node-version: 22 - cache: "yarn" cache-dependency-path: yarn.lock
🧹 Nitpick comments (3)
.github/actions/yarn-install/action.yml (1)
29-35: Add explicit shell specification to retry wrapper.The previous version (line 15) specified
shell: bash. The retry wrapper at lines 30–35 does not include an explicit shell specification. Whilenick-fields/retry@v3may default to bash, explicitly declaring the shell ensures consistency and clarity.- name: Install dependencies uses: nick-fields/retry@v3 with: timeout_minutes: 10 max_attempts: 3 retry_wait_seconds: 5 + shell: bash command: yarn install --immutable.github/workflows/mobile-deploy.yml (1)
1060-1076: Document NDK/CMake retry parameter rationale.Both NDK and CMake installation use
max_attempts: 5, while CI workflows (mobile-ci.yml, mobile-e2e.yml) usemax_attempts: 3. If this higher count is intentional for production deployments, add a comment. Otherwise, standardize to 3 across all workflows.Add inline comments clarifying the production-vs-CI retry strategy:
- name: Install NDK if: inputs.platform != 'ios' && steps.ndk-cache.outputs.cache-hit != 'true' uses: nick-fields/retry@v3 with: timeout_minutes: 15 max_attempts: 5 # Higher retry count for production reliability retry_wait_seconds: 10 command: sdkmanager "ndk;${{ env.ANDROID_NDK_VERSION }}".github/workflows/mobile-sdk-demo-e2e.yml (1)
323-326: Apply secret masking consistently across workflows.Lines 323–326 properly mask the
SELFXYZ_INTERNAL_REPO_PATusing::add-mask::, which is good practice. However,mobile-ci.ymlandmobile-e2e.ymlpass the same secret to retry steps without this explicit masking. GitHub Actions typically handles secret masking automatically, but consider applying this pattern uniformly for defense-in-depth.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/actions/yarn-install/action.yml(1 hunks).github/workflows/mobile-ci.yml(2 hunks).github/workflows/mobile-deploy.yml(1 hunks).github/workflows/mobile-e2e.yml(5 hunks).github/workflows/mobile-sdk-demo-e2e.yml(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.github/workflows/**/*.{yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
.github/workflows/**/*.{yml,yaml}: In GitHub workflows, use the shared composite actions in .github/actions for dependency caching instead of calling actions/cache directly
Use the cache-yarn composite action for Yarn dependency caching in workflows
Use the cache-bundler composite action for Ruby gems caching in workflows
Use the cache-gradle composite action for Gradle caching in workflows
Use the cache-pods composite action for CocoaPods caching in workflows
Files:
.github/workflows/mobile-sdk-demo-e2e.yml.github/workflows/mobile-ci.yml.github/workflows/mobile-e2e.yml.github/workflows/mobile-deploy.yml
🧠 Learnings (18)
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Test, build, and deploy scripts (`yarn test`, `yarn ios`, `yarn test:e2e:ios`, Fastlane, etc.) must be used for automation.
Applied to files:
.github/workflows/mobile-sdk-demo-e2e.yml.github/workflows/mobile-ci.yml.github/workflows/mobile-e2e.yml
📚 Learning: 2025-10-08T20:23:58.783Z
Learnt from: transphorm
PR: selfxyz/self#1244
File: .github/workflows/mobile-deploy.yml:774-776
Timestamp: 2025-10-08T20:23:58.783Z
Learning: In the selfxyz/self repository, for the mobile deployment workflow (.github/workflows/mobile-deploy.yml):
- iOS builds cache Ruby gems at `app/ios/vendor/bundle`
- Android builds cache Ruby gems at `app/vendor/bundle`
- These paths should be used consistently within their respective build jobs
Applied to files:
.github/workflows/mobile-sdk-demo-e2e.yml.github/workflows/mobile-ci.yml.github/workflows/mobile-e2e.yml.github/workflows/mobile-deploy.yml
📚 Learning: 2025-09-22T11:10:57.879Z
Learnt from: CR
PR: selfxyz/self#0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: Applies to app/ios/**/*.{m,mm,swift} : Document complex native iOS module changes in the PR
Applied to files:
.github/workflows/mobile-sdk-demo-e2e.yml.github/workflows/mobile-ci.yml.github/workflows/mobile-e2e.yml
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Run yarn nice, yarn types, and yarn test before commits and PRs
Applied to files:
.github/workflows/mobile-sdk-demo-e2e.yml.github/workflows/mobile-e2e.yml
📚 Learning: 2025-09-22T11:10:57.879Z
Learnt from: CR
PR: selfxyz/self#0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: Run unit tests and ensure they pass (yarn test) before PRs
Applied to files:
.github/workflows/mobile-sdk-demo-e2e.yml.github/workflows/mobile-e2e.yml
📚 Learning: 2025-09-22T11:10:57.879Z
Learnt from: CR
PR: selfxyz/self#0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: Optionally use Watchman and run yarn install at repo root
Applied to files:
.github/workflows/mobile-sdk-demo-e2e.yml.github/workflows/mobile-e2e.yml.github/actions/yarn-install/action.yml
📚 Learning: 2025-09-22T11:10:57.879Z
Learnt from: CR
PR: selfxyz/self#0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: Run linting/formatting and ensure no new issues (yarn nice, yarn lint) before PRs
Applied to files:
.github/workflows/mobile-sdk-demo-e2e.yml.github/workflows/mobile-e2e.yml
📚 Learning: 2025-10-04T05:29:43.587Z
Learnt from: CR
PR: selfxyz/self#0
File: AGENTS.md:0-0
Timestamp: 2025-10-04T05:29:43.587Z
Learning: Applies to .github/workflows/**/*.{yml,yaml} : Use the cache-yarn composite action for Yarn dependency caching in workflows
Applied to files:
.github/workflows/mobile-sdk-demo-e2e.yml.github/workflows/mobile-ci.yml.github/workflows/mobile-e2e.yml.github/actions/yarn-install/action.yml
📚 Learning: 2025-09-22T11:10:57.879Z
Learnt from: CR
PR: selfxyz/self#0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: Applies to app/**/*.{ts,tsx} : Type checking must pass before PRs (yarn types)
Applied to files:
.github/workflows/mobile-sdk-demo-e2e.yml.github/workflows/mobile-e2e.yml
📚 Learning: 2025-09-22T11:10:57.879Z
Learnt from: CR
PR: selfxyz/self#0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: Generate build artifacts successfully after PR creation
Applied to files:
.github/workflows/mobile-sdk-demo-e2e.yml.github/workflows/mobile-e2e.yml
📚 Learning: 2025-09-22T11:10:57.879Z
Learnt from: CR
PR: selfxyz/self#0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: Ensure app builds succeed for iOS, Android, and Web before PRs
Applied to files:
.github/workflows/mobile-sdk-demo-e2e.yml.github/workflows/mobile-ci.yml.github/workflows/mobile-e2e.yml
📚 Learning: 2025-10-04T05:29:43.587Z
Learnt from: CR
PR: selfxyz/self#0
File: AGENTS.md:0-0
Timestamp: 2025-10-04T05:29:43.587Z
Learning: Applies to .github/workflows/**/*.{yml,yaml} : Use the cache-pods composite action for CocoaPods caching in workflows
Applied to files:
.github/workflows/mobile-ci.yml
📚 Learning: 2025-10-04T05:29:43.587Z
Learnt from: CR
PR: selfxyz/self#0
File: AGENTS.md:0-0
Timestamp: 2025-10-04T05:29:43.587Z
Learning: Applies to .github/workflows/**/*.{yml,yaml} : In GitHub workflows, use the shared composite actions in .github/actions for dependency caching instead of calling actions/cache directly
Applied to files:
.github/workflows/mobile-ci.yml
📚 Learning: 2025-09-22T11:10:57.879Z
Learnt from: CR
PR: selfxyz/self#0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: Applies to app/android/**/*.{kt,java} : Document complex native Android module changes in the PR
Applied to files:
.github/workflows/mobile-e2e.yml.github/workflows/mobile-deploy.yml
📚 Learning: 2025-09-22T11:10:57.879Z
Learnt from: CR
PR: selfxyz/self#0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: Applies to app/**/*.{ios,android,web}.{ts,tsx,js,jsx} : Explain platform-specific code paths in the PR description (files with .ios, .android, or .web extensions)
Applied to files:
.github/workflows/mobile-e2e.yml
📚 Learning: 2025-09-22T11:10:57.879Z
Learnt from: CR
PR: selfxyz/self#0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: For iOS development, install Xcode/CLT and CocoaPods, then run pod install
Applied to files:
.github/workflows/mobile-e2e.yml
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/artifacts/** : Place artifact and manifest management in packages/mobile-sdk-alpha/src/artifacts/
Applied to files:
.github/workflows/mobile-deploy.yml
📚 Learning: 2025-07-29T01:08:28.530Z
Learnt from: transphorm
PR: selfxyz/self#795
File: app/android/app/build.gradle:157-158
Timestamp: 2025-07-29T01:08:28.530Z
Learning: For this React Native project, the team prefers build flexibility over fail-fast behavior for release builds in app/android/app/build.gradle. They intentionally allow fallback to debug signing for local development runs, relying on Google Play Console validation to catch any improperly signed releases during upload.
Applied to files:
.github/workflows/mobile-deploy.yml
⏰ 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). (6)
- GitHub Check: type-check
- GitHub Check: android-build-test
- GitHub Check: e2e-ios
- GitHub Check: build-deps
- GitHub Check: analyze-android
- GitHub Check: analyze-ios
🔇 Additional comments (1)
.github/workflows/mobile-e2e.yml (1)
337-346: Verify pod-install-with-cache-fix.sh availability in retry context.Line 345 executes
bundle exec bash scripts/pod-install-with-cache-fix.shwithin the retry wrapper. Ensure this script exists and is accessible from theapp/iosdirectory during workflow execution.Verify that the script path is correct relative to the
cd app/ioscommand and that SELFXYZ_INTERNAL_REPO_PAT is properly passed through.
| id: ndk-cache | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: ${{ env.ANDROID_HOME }}/ndk/${{ env.ANDROID_NDK_VERSION }} | ||
| key: ${{ runner.os }}-ndk-${{ env.ANDROID_NDK_VERSION }} | ||
| - name: Install NDK | ||
| run: sdkmanager "ndk;${{ env.ANDROID_NDK_VERSION }}" | ||
| if: steps.ndk-cache.outputs.cache-hit != 'true' | ||
| uses: nick-fields/retry@v3 | ||
| with: | ||
| timeout_minutes: 15 | ||
| max_attempts: 3 | ||
| retry_wait_seconds: 10 | ||
| command: sdkmanager "ndk;${{ env.ANDROID_NDK_VERSION }}" |
There was a problem hiding this comment.
Inconsistent NDK retry configuration across workflows.
This file uses max_attempts: 3 for NDK installation (line 451), while mobile-deploy.yml (line 1065) uses max_attempts: 5. Standardize retry parameters across workflows unless deployment context justifies higher retry count.
Consider standardizing NDK retry attempts to 3 or 5 consistently, or add a comment explaining why deployment workflows need more retries.
🤖 Prompt for AI Agents
In .github/workflows/mobile-ci.yml around lines 441 to 453 the NDK install step
uses retry configuration max_attempts: 3 which is inconsistent with
mobile-deploy.yml (max_attempts: 5); update this workflow to use the same retry
count as mobile-deploy.yml (or change both to the chosen standard) and/or add a
terse comment explaining why the deploy workflow needs a different retry count
(if it does), ensuring both files use the agreed value and consistent retry
settings.
| uses: nick-fields/retry@v3 | ||
| with: | ||
| timeout_minutes: 20 | ||
| max_attempts: 3 | ||
| retry_wait_seconds: 10 | ||
| command: | | ||
| if [ -n "${SELFXYZ_INTERNAL_REPO_PAT}" ]; then | ||
| echo "🔑 Using SELFXYZ_INTERNAL_REPO_PAT for private pod access" | ||
| echo "::add-mask::${SELFXYZ_INTERNAL_REPO_PAT}" | ||
| fi | ||
| cd packages/mobile-sdk-demo/ios | ||
| pod install | ||
| env: | ||
| SELFXYZ_INTERNAL_REPO_PAT: ${{ secrets.SELFXYZ_INTERNAL_REPO_PAT }} | ||
| GIT_TERMINAL_PROMPT: 0 |
There was a problem hiding this comment.
Use consistent pod installation script across workflows.
This file uses direct pod install (line 328), while mobile-ci.yml and mobile-e2e.yml invoke scripts/pod-install-with-cache-fix.sh. If this script contains important workarounds or fixes, the SDK demo should use it too. If not, document why the demo uses a simpler approach.
Align the pod install approach:
Option A: Use the centralized script (recommended for consistency):
command: |
if [ -n "${SELFXYZ_INTERNAL_REPO_PAT}" ]; then
echo "🔑 Using SELFXYZ_INTERNAL_REPO_PAT for private pod access"
echo "::add-mask::${SELFXYZ_INTERNAL_REPO_PAT}"
fi
cd packages/mobile-sdk-demo/ios
- pod install
+ bundle exec bash ../../app/scripts/pod-install-with-cache-fix.shOption B: Or document why the demo uses direct pod install in a comment.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
.github/workflows/mobile-sdk-demo-e2e.yml around lines 317 to 331: the workflow
runs a direct `pod install` in packages/mobile-sdk-demo/ios which differs from
other workflows that call scripts/pod-install-with-cache-fix.sh; update the
workflow to call the centralized script for consistency and to pick up any
cache/fix workarounds (replace the `pod install` line with a call to
scripts/pod-install-with-cache-fix.sh and ensure the script is executable and
run from repo root), or if you intentionally want the simpler install, add a
short inline comment above the command explaining why the centralized script is
not used and confirm there are no required workarounds missing.
Summary by CodeRabbit