Skip to content

Conversation

@transphorm
Copy link
Member

@transphorm transphorm commented Aug 18, 2025

Issue seems be related to the github runner

Summary

  • trim redundant caches and setup steps from iOS E2E workflow
  • focus Yarn install on mobile app workspace

Testing

  • yarn workspaces foreach -p -v --topological-dev --since=HEAD run nice --if-present
  • yarn lint
  • yarn build
  • yarn workspace @selfxyz/contracts build (fails: Invalid account in Hardhat config)
  • yarn types
  • yarn test (fails: exit code 37/1 in circuits/contracts)

https://chatgpt.com/codex/tasks/task_b_68a2acabdf8c832db13a667f7eee6d9f

Summary by CodeRabbit

  • Chores

    • Standardized Xcode to 16.4 across CI, E2E, and deploy pipelines and added a Configure Xcode step to address macOS iOS SDK issues.
    • Added Android NDK (27.0.11718014), Java, and Ruby setup, NDK caching, and earlier Ruby dependency install in mobile CI.
    • Moved mobile dependency steps into app subdirectories and improved working-directory handling.
    • Added Node.js type definitions for development.
    • CI/E2E workflows now trigger and gate jobs based on downstream workflow results and per-project change detection.
  • New Features

    • Per-project iOS support: dynamic workspace/scheme handling, workspace-aware build flow, pre-build validations, and enhanced build logs.
    • Automated iOS simulator provisioning that creates/boots simulators and exposes simulator identifiers for downstream steps.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 18, 2025

Walkthrough

Workflows updated to centralize XCODE_VERSION, add NDK and Android toolchain setup and caching, and introduce per-project iOS E2E wiring (change detection, workspace/scheme validation, pod install, simulator creation/selection, and exported simulator IDs). Added devDependency @types/node to common/package.json.

Changes

Cohort / File(s) Summary
E2E iOS workflow
.github/workflows/mobile-e2e.yml
Added XCODE_VERSION; introduced changes job computing per-project change flags (ios/android/shared) and exposing outputs; e2e-ios now needs changes and gates run on ios/shared outputs; added IOS_PROJECT_NAME/IOS_PROJECT_SCHEME; replaced Set up Xcode with inline Configure Xcode path; added Install iOS dependencies (pod install) in app/ios; added Verify iOS Runtime and Setup iOS Simulator (list/create/boot/wait, export IOS_SIMULATOR_ID/IOS_SIMULATOR_NAME); Build iOS App computes WORKSPACE_PATH from IOS_PROJECT_NAME, validates workspace and scheme, enumerates schemes, and builds to the simulator destination; enhanced logging.
Mobile CI workflow
.github/workflows/mobile-ci.yml
Added ANDROID_NDK_VERSION and XCODE_VERSION to env; Set up Xcode now reads ${{ env.XCODE_VERSION }} and an inline Configure Xcode path step added; added Set up Ruby (ruby/setup-ruby@v1) and Install Ruby Dependencies (working-directory: ./app); added Java setup (actions/setup-java@v4), Android SDK setup, NDK cache (actions/cache@v4) and Install NDK (sdkmanager); moved iOS pod/workspace verification to run in working-directory ./app/ios; adjusted toolchain ordering and caching.
Deploy workflow
.github/workflows/mobile-deploy.yml
Added XCODE_VERSION to env; replaced Set up Xcode action with inline Configure Xcode path step (xcode-select to /Applications/Xcode_${XCODE_VERSION}.app and verify xcodebuild/xcode-select outputs).
Package devDependency
common/package.json
Added devDependency @types/node: ^22.0.0.

Sequence Diagram(s)

sequenceDiagram
  participant GH as GitHub Actions (e2e-ios)
  participant Repo as Repository FS
  participant Simctl as simctl / runtimes
  participant Xcode as xcodebuild
  participant E2E as E2E Test Runner

  GH->>Repo: Read IOS_PROJECT_NAME & IOS_PROJECT_SCHEME
  GH->>Repo: Validate workspace exists (app/ios/${IOS_PROJECT_NAME}.xcworkspace)
  GH->>Repo: List schemes -> validate IOS_PROJECT_SCHEME
  GH->>Simctl: List runtimes (Verify iOS Runtime)
  alt runtime usable
    GH->>Simctl: Find or create simulator -> boot -> wait for readiness
    Simctl-->>GH: Return IOS_SIMULATOR_ID / IOS_SIMULATOR_NAME
    GH->>GH: Export simulator env vars
  end
  GH->>Xcode: xcodebuild -workspace WORKSPACE_PATH -scheme IOS_PROJECT_SCHEME -destination id=IOS_SIMULATOR_ID
  Xcode-->>Repo: Produce build artifacts
  GH->>E2E: Run tests on simulator
  E2E-->>GH: Return results
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • aaronmgdr
  • remicolin

Poem

Xcode paths set true, simulators awake,
Pods install, NDK cached — a smoother build to make.
Schemes confirmed, simulator IDs unfurled,
CI hums along — mobile tests in the world. 🚀


📜 Recent 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 2965526 and cce8a16.

📒 Files selected for processing (1)
  • .github/workflows/mobile-e2e.yml (7 hunks)
⏰ 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: run_circuit_tests
🔇 Additional comments (1)
.github/workflows/mobile-e2e.yml (1)

215-229: Xcode 16.4 pin + explicit xcode-select mitigates macOS 15 SDK breakage

Good defensive setup: pinning via setup-xcode and switching to /Applications/Xcode_16.4.app addresses the runner-images iOS SDK issue.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/find-opportunities-to-speed-up-ios-e2e-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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@giga-agent
Copy link

giga-agent bot commented Aug 18, 2025

Giga Summary

• Streamline iOS E2E workflow by removing redundant caches and setup steps.
• Impact: Improved efficiency and reduced execution time for iOS E2E tests.
• Changes:

  • Removed redundant caches and setup steps from the iOS E2E workflow.
  • Focused Yarn install on the mobile app workspace.
    • Focus:
  • Verify that the streamlined workflow does not introduce any regressions or issues in the iOS E2E tests.

Quality Assessment

• Streamlined workflow and cache cleanup enhance efficiency.
• Potential build reliability concerns need addressing.
• Recommend approval with condition to monitor build stability post-merge.

Quality Score: 7/10 (Threshold: 7/10)

💬 Detailed comments have been added to specific code changes.

Copy link

@giga-agent giga-agent bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Detailed code review comments

@transphorm transphorm marked this pull request as draft August 18, 2025 05:05
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🔭 Outside diff range comments (2)
.github/workflows/mobile-e2e.yml (2)

197-199: Avoid curl | bash supply-chain risk; pin and verify Maestro installer.

Piping remote scripts to shell is a medium+ risk in CI. Prefer a pinned installer with checksum verification or a package manager.

Options:

  • Preferred: Install via Homebrew (version-pinned):
-      - name: Install Maestro
-        if: steps.cache-maestro.outputs.cache-hit != 'true'
-        run: curl -Ls "https://get.maestro.mobile.dev" | bash
+      - name: Install Maestro
+        if: steps.cache-maestro.outputs.cache-hit != 'true'
+        run: |
+          brew update
+          brew install mobile-dev-inc/tap/[email protected]
  • Or, download a specific release asset and verify checksum before executing (store/update the expected sha256 securely in the repo):
-      - name: Install Maestro
-        if: steps.cache-maestro.outputs.cache-hit != 'true'
-        run: curl -Ls "https://get.maestro.mobile.dev" | bash
+      - name: Install Maestro
+        if: steps.cache-maestro.outputs.cache-hit != 'true'
+        run: |
+          VERSION="1.41.0"
+          curl -Lso maestro-install.sh "https://get.maestro.mobile.dev"
+          echo "${MAESTRO_INSTALL_SHA256}  maestro-install.sh" | shasum -a 256 -c -
+          bash maestro-install.sh

286-293: Hard-coded iOS runtime string makes simulator creation brittle; use the detected RUNTIME_ID.

simctl create ... "iOS17.5" will break as macOS runners update runtimes. You already compute RUNTIME_ID; use it with a fallback.

Apply this diff:

-            # Create a new iPhone SE (3rd generation) simulator with the latest iOS version
-            xcrun simctl create "iPhone SE (3rd generation)" "iPhone SE (3rd generation)" "iOS17.5" || {
+            # Create a new iPhone SE (3rd generation) simulator with the detected latest iOS runtime
+            if [ -n "$RUNTIME_ID" ]; then
+              xcrun simctl create "iPhone SE (3rd generation)" "iPhone SE (3rd generation)" "$RUNTIME_ID" || {
+                echo "❌ Failed to create iPhone SE (3rd generation) simulator with $RUNTIME_ID"
+                echo "Trying to create any iPhone SE simulator..."
+                xcrun simctl create "iPhone SE" "iPhone SE" "$RUNTIME_ID" || {
+                  echo "❌ Failed to create simulator"
+                  exit 1
+                }
+              }
+            else
+              echo "⚠️ No RUNTIME_ID detected; attempting best-effort create with default identifier"
+              xcrun simctl create "iPhone SE (3rd generation)" "iPhone SE (3rd generation)" || true
+            fi
-              echo "❌ Failed to create iPhone SE (3rd generation) simulator"
-              echo "Trying to create any iPhone SE simulator..."
-              xcrun simctl create "iPhone SE" "iPhone SE" "iOS17.5" || {
-                echo "❌ Failed to create simulator"
-                exit 1
-              }
-            }
🧹 Nitpick comments (2)
.github/workflows/mobile-e2e.yml (2)

189-189: Workspace focus can prune deps needed by native iOS builds — verify coverage or add fallback.

yarn workspaces focus @selfxyz/mobile-app --mode=skip-build will prune to the minimal set for that workspace. Ensure all native modules and scripts invoked by Pods/Xcode are reachable (e.g., react-native CLI, codegen, Metro config). If any are dev-only in other workspaces, they may be excluded.

Suggested guard to fail fast if key deps are missing:

-      - run: yarn workspaces focus @selfxyz/mobile-app --mode=skip-build --silent
+      - run: |
+          yarn workspaces focus @selfxyz/mobile-app --mode=skip-build --silent
+          # Sanity checks for commonly needed deps in native builds
+          yarn -s why react-native || { echo "❌ react-native not resolved"; exit 1; }
+          yarn -s why @react-native/codegen || true

205-207: Update Node modules cache key to use the root lockfile

The workflow is hashing app/yarn.lock, which doesn’t exist in this repo—only the root yarn.lock is present. As a result, the cache key never changes when dependencies update, causing unnecessary cache misses (and slower CI runs).

Please update the cache key in .github/workflows/mobile-e2e.yml (lines 205–207):

diff --git a/.github/workflows/mobile-e2e.yml b/.github/workflows/mobile-e2e.yml
index abcdef1..2345678 100644
--- a/.github/workflows/mobile-e2e.yml
+++ b/.github/workflows/mobile-e2e.yml
@@ -205,7 +205,7 @@ jobs:
path: ~/.npm
- name: Cache Node modules
uses: actions/cache@v3

  •    with:
    
  •      key: ${{ runner.os }}-node-${{ env.NODE_VERSION_SANITIZED }}-${{ hashFiles('app/yarn.lock') }}
    
  •    with:
    
  •      key: ${{ runner.os }}-node-${{ env.NODE_VERSION_SANITIZED }}-${{ hashFiles('yarn.lock') }}
         restore-keys: |
           ${{ runner.os }}-node-${{ env.NODE_VERSION_SANITIZED }}-
           ${{ runner.os }}-node-
    

This ensures the cache key reflects changes to your actual lockfile.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 691e8b1 and 0ca0fcd.

📒 Files selected for processing (1)
  • .github/workflows/mobile-e2e.yml (1 hunks)
🔇 Additional comments (1)
.github/workflows/mobile-e2e.yml (1)

186-189: Composite action already includes the lockfile hash—no changes needed

The cache-yarn composite action uses hashFiles(inputs.lock-file) (defaulting to yarn.lock) in its cache key. Your workflow’s cache-version input is correctly combined with that hash, so you don’t need to append hashFiles('**/yarn.lock') yourself.

Likely an incorrect or invalid review comment.

uses: maxim-lobanov/setup-xcode@v1
with:
xcode-version: "16.4"
xcode-version: "16.2"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixes incorrect upgrade from previous pr

@transphorm transphorm marked this pull request as ready for review August 18, 2025 05:22
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🔭 Outside diff range comments (1)
.github/workflows/mobile-e2e.yml (1)

220-247: Hardcoded simulator runtime (“iOS17.5”) will fail or flake with Xcode 16.2 images

You detect the latest available runtime but don’t persist or use it; then creation hardcodes “iOS17.5”, which likely isn’t present on current images (16.2 typically ships iOS 18.x). Result: simulator creation failures. Persist the detected runtime to GITHUB_ENV and use it during sim creation.

Persist the runtime (end of “Install iOS Runtime” step):

           if [ -n "$RUNTIME_ID" ] && [[ "$RUNTIME_ID" == com.apple.CoreSimulator.SimRuntime.iOS-* ]]; then
             echo "✅ Latest iOS runtime: $RUNTIME_ID"
           else
             echo "⚠️ Could not determine an iOS runtime identifier"
           fi
+          # Persist for later steps
+          if [ -n "$RUNTIME_ID" ]; then
+            echo "IOS_RUNTIME_ID=$RUNTIME_ID" >> "$GITHUB_ENV"
+          fi

Use the persisted runtime when creating the simulator:

-            xcrun simctl create "iPhone SE (3rd generation)" "iPhone SE (3rd generation)" "iOS17.5" || {
+            RUNTIME_TO_USE="${IOS_RUNTIME_ID}"
+            if [ -z "$RUNTIME_TO_USE" ]; then
+              RUNTIME_TO_USE=$(xcrun simctl list -j runtimes \
+                | jq -r '.runtimes[] | select(.platform=="iOS" and .isAvailable==true) | .identifier' \
+                | sort -V | tail -1)
+            fi
+            xcrun simctl create "iPhone SE (3rd generation)" "iPhone SE (3rd generation)" "$RUNTIME_TO_USE" || {

Also applies to: 286-299

🧹 Nitpick comments (3)
.github/workflows/mobile-deploy.yml (1)

118-123: Pin runner to a macOS image that guarantees Xcode 16.2 availability

You’ve downgraded Xcode to 16.2, but the job runs on macos-latest, which can drift to an image where 16.2 isn’t installed. setup-xcode can only select preinstalled versions; if 16.2 isn’t present, the job will fail. Pin to macos-14 (which has 16.2 available) to avoid flaky/broken builds.

Suggested change (outside the selected lines):

 jobs:
   build-ios:
-    runs-on: macos-latest
+    runs-on: macos-14
.github/workflows/mobile-e2e.yml (2)

186-190: Use immutable installs with Yarn focus to preserve reproducibility

Replacing “yarn install --immutable” with “yarn workspaces focus …” removes the frozen lockfile check. That can silently drift dependencies and destabilize E2E. Also ensure focus actually performs installation for the target workspace in CI.

Suggested change:

-      - run: yarn workspaces focus @selfxyz/mobile-app
+      - run: yarn workspaces focus @selfxyz/mobile-app --all --immutable

Verification checklist:

  • Confirm your .yarnrc.yml uses nodeLinker: node-modules if you rely on app/node_modules cache.
  • Ensure the focus command completes an install; otherwise, keep a preceding “yarn install --immutable” at repo root.

202-205: Align runner image with Xcode 16.2 to avoid version selection failures

This job uses macos-latest (Line 161) and selects Xcode 16.2. If the latest image doesn’t include 16.2, setup-xcode will fail. Pin to macos-14 for stability or set xcode-version to “latest-stable” if you can absorb the newer toolchain.

Suggested change (outside the selected lines):

 e2e-ios:
-  runs-on: macos-latest
+  runs-on: macos-14
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 0ca0fcd and a528c4c.

📒 Files selected for processing (4)
  • .github/workflows/mobile-ci.yml (1 hunks)
  • .github/workflows/mobile-deploy.yml (1 hunks)
  • .github/workflows/mobile-e2e.yml (2 hunks)
  • app/fastlane/Fastfile (1 hunks)
⏰ 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). (3)
  • GitHub Check: e2e-ios
  • GitHub Check: analyze-android
  • GitHub Check: analyze-ios
🔇 Additional comments (1)
.github/workflows/mobile-ci.yml (1)

124-126: Xcode 16.2 selection looks good and aligns with other workflows

The version pin is consistent with deploy/e2e. Since this job already runs on macos-14, setup-xcode should reliably select 16.2.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
app/fastlane/Fastfile (1)

206-217: Robust workspace resolution: good improvement and aligns with prior feedback

Choosing between scheme- and project-named .xcworkspace reduces CI flakiness when names diverge. This addresses the earlier review concern about hard-coding the scheme as the workspace filename.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between a528c4c and 6e7c05c.

📒 Files selected for processing (1)
  • app/fastlane/Fastfile (1 hunks)
⏰ 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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🔭 Outside diff range comments (3)
.github/workflows/mobile-ci.yml (3)

11-17: Workflow will not trigger on edits to this file

The push path filter omits this workflow file. Changes to .github/workflows/mobile-ci.yml won’t trigger the workflow, which can silently break CI.

Apply one of these to ensure triggers fire on workflow changes:

   push:
     paths:
       - "common/**"
       - "app/**"
-      - ".github/workflows/app.yml"
+      - ".github/workflows/app.yml"
+      - ".github/workflows/mobile-ci.yml"
       - ".github/actions/**"

Or more generally:

   push:
     paths:
       - "common/**"
       - "app/**"
-      - ".github/workflows/app.yml"
+      - ".github/workflows/**"
       - ".github/actions/**"

161-178: Unreachable/inverted Pods cleanup logic in cache verification

Inside the “cache restored successfully” branch (where Pods is known to be non-empty), you then check for an empty/nonexistent Pods dir and attempt to clear it. That condition can never be true here, so corrupted/empty caches won’t be cleaned as intended.

Simplify and make the cleanup effective:

       - name: Verify CocoaPods Cache
         run: |
           echo "Checking CocoaPods cache status..."
-          if [ -d "app/ios/Pods" ] && [ "$(ls -A app/ios/Pods)" ]; then
-            echo "✅ CocoaPods cache restored successfully"
-            ls -la app/ios/Pods/ | head -10
-
-            # Check if key files exist
-            if [ ! -f "app/ios/Podfile.lock" ]; then
-              echo "⚠️ Podfile.lock is missing - pods cache may be stale"
-            fi
-            # If Pods directory exists but is incomplete, clear only Pods; keep Podfile.lock
-            if [ ! -d "app/ios/Pods" ] || [ -z "$(ls -A app/ios/Pods 2>/dev/null)" ]; then
-              echo "⚠️ Pods directory incomplete - clearing Pods (preserving Podfile.lock)"
-              rm -rf app/ios/Pods
-            fi
-          else
-            echo "⚠️ CocoaPods cache is empty or missing - will install fresh"
-          fi
+          if [ -d "app/ios/Pods" ]; then
+            if [ -n "$(ls -A app/ios/Pods 2>/dev/null)" ]; then
+              echo "✅ CocoaPods cache restored successfully"
+              ls -la app/ios/Pods/ | head -10
+              if [ ! -f "app/ios/Podfile.lock" ]; then
+                echo "⚠️ Podfile.lock is missing - pods cache may be stale"
+              fi
+            else
+              echo "⚠️ Pods directory exists but is empty - clearing Pods"
+              rm -rf app/ios/Pods
+            fi
+          else
+            echo "⚠️ CocoaPods cache is missing - will install fresh"
+          fi

200-229: bundle exec pod install likely fails due to Gemfile locality; preserve Podfile.lock for reproducibility

This step runs in ./app/ios, but bundler was installed using the Gemfile in ./app. Running bundle exec from ios without telling Bundler where the Gemfile is will error (“Could not locate Gemfile”). Also, deleting Podfile.lock on cache misses undermines reproducible builds and cache keys.

  • Point Bundler to ../Gemfile while remaining in ios for CocoaPods.
  • Avoid removing Podfile.lock; clear only Pods when needed.
           # 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
+            rm -rf Pods
           fi

           # Install pods
-          bundle exec pod install --silent || { echo "❌ Pod install failed"; exit 1; }
+          BUNDLE_GEMFILE=../Gemfile bundle exec pod install --silent || { echo "❌ Pod install failed"; exit 1; }
           echo "✅ Pods installed successfully"

If you prefer, you can set the env at the step level instead of inline:

env:
  BUNDLE_GEMFILE: ../Gemfile

in the same step (keeps the command cleaner).

🧹 Nitpick comments (1)
.github/workflows/mobile-ci.yml (1)

135-135: Redundant second checkout increases runtime

A second actions/checkout immediately after Ruby setup is unnecessary and adds I/O overhead.

-      - uses: actions/checkout@v4
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 6e7c05c and 64c14ed.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (3)
  • .github/workflows/mobile-ci.yml (4 hunks)
  • .github/workflows/mobile-e2e.yml (1 hunks)
  • common/package.json (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • common/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/mobile-e2e.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). (2)
  • GitHub Check: run_circuit_tests
  • GitHub Check: build
🔇 Additional comments (2)
.github/workflows/mobile-ci.yml (2)

126-127: Good call aligning Xcode to 16.2

This helps eliminate version drift across jobs and reduces cache thrash on macOS runners.


188-190: Confirm yarn install scope matches PR goal (mobile-only install)

The shared ./.github/actions/yarn-install is invoked without a working-directory or focus/frozen settings. Depending on how that action is implemented, it may install all workspaces at repo root, counter to the stated objective to limit installs to the mobile app workspace.

  • If the action already focuses the app workspace, ignore this.
  • If not, consider one of:
    • Run yarn from ./app with a workspace-focused strategy your repo supports (e.g., workspace-tools plugin: yarn workspaces focus @selfxyz/app --production).
    • Or keep root install but gate non-mobile workspaces via --since/filters if you’re on Yarn 4 with constraints.

I can review ./.github/actions/yarn-install if you want to ensure it’s properly scoped.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🔭 Outside diff range comments (8)
.github/workflows/mobile-ci.yml (3)

220-224: Do not delete Podfile.lock in CI; it breaks reproducible and secure builds

Removing Podfile.lock undermines determinism and can introduce supply-chain drift. Keep the lockfile and only clear Pods when needed.

Apply this diff:

-          if [ ! -d "Pods" ] || [ -z "$(ls -A Pods 2>/dev/null)" ]; then
-            echo "Cleaning empty/corrupted Pods directory..."
-            rm -rf Pods Podfile.lock
-          fi
+          if [ ! -d "Pods" ] || [ -z "$(ls -A Pods 2>/dev/null)" ]; then
+            echo "Cleaning empty/corrupted Pods directory (preserving Podfile.lock)..."
+            rm -rf Pods
+          fi

240-243: Hard-coded Pods-Self path is brittle and likely to fail on different project names

Checking for Pods/Target Support Files/Pods-Self ties the workflow to a specific iOS target name. This will break if the Pod target name differs (e.g., when using a different workspace/scheme).

Apply this diff to verify presence generically:

-          if [ ! -f "Pods/Target Support Files/Pods-Self/Pods-Self.debug.xcconfig" ]; then
-            echo "❌ Key CocoaPods configuration file missing"
-            exit 1
-          fi
+          # Verify at least one Pods xcconfig exists for any target
+          if ! ls "Pods/Target Support Files"/*/Pods-*.debug.xcconfig >/dev/null 2>&1; then
+            echo "❌ No CocoaPods xcconfig found under 'Pods/Target Support Files/*/Pods-*.debug.xcconfig'"
+            echo "Available target support files:"
+            find "Pods/Target Support Files" -maxdepth 2 -type f 2>/dev/null | head -50 || true
+            exit 1
+          fi

251-255: Workspace name is hard-coded to OpenPassport; auto-detect or parameterize to avoid false failures

The e2e workflow now supports dynamic project/workspace names; this step still hard-codes OpenPassport.xcworkspace and will fail for projects named differently (e.g., “Self”).

Apply this diff to auto-detect the workspace:

-          if [ ! -f "OpenPassport.xcworkspace/contents.xcworkspacedata" ]; then
-            echo "❌ OpenPassport.xcworkspace is missing or corrupted"
-            exit 1
-          fi
+          WORKSPACE_FILE="$(find . -maxdepth 1 -name '*.xcworkspace' -type d -print -quit)/contents.xcworkspacedata"
+          if [ -z "$WORKSPACE_FILE" ] || [ ! -f "$WORKSPACE_FILE" ]; then
+            echo "❌ No .xcworkspace found or workspace is corrupted"
+            echo "Available workspaces:"
+            find . -maxdepth 1 -name '*.xcworkspace' -type d
+            exit 1
+          fi
.github/workflows/mobile-e2e.yml (5)

205-208: Avoid curl | bash for Maestro; pin versioned binary to reduce supply-chain risk

Piping a remote script into bash is a security risk and may ignore your MAESTRO_VERSION. Fetch a pinned release and install the binary explicitly.
[security]

Apply this diff:

-      - name: Install Maestro
-        if: steps.cache-maestro.outputs.cache-hit != 'true'
-        run: curl -Ls "https://get.maestro.mobile.dev" | bash
+      - name: Install Maestro (pinned)
+        if: steps.cache-maestro.outputs.cache-hit != 'true'
+        run: |
+          set -euo pipefail
+          VERSION="${MAESTRO_VERSION}"
+          OS="darwin"
+          ARCH="arm64"
+          URL="https://github.com/mobile-dev-inc/maestro/releases/download/${VERSION}/maestro-${OS}-${ARCH}.zip"
+          echo "Downloading Maestro ${VERSION} from ${URL}"
+          TMP="$(mktemp -d)"
+          curl -L "$URL" -o "$TMP/maestro.zip"
+          unzip -q "$TMP/maestro.zip" -d "$TMP"
+          mkdir -p "$HOME/.maestro/bin"
+          mv "$TMP/maestro" "$HOME/.maestro/bin/maestro"
+          chmod +x "$HOME/.maestro/bin/maestro"

Note: If you need universal/Intel runners, detect architecture via uname -m and map accordingly.


294-298: Pod install without Ruby/Bundle setup will be flaky on runners

There’s no Ruby setup or bundle install. On macOS runners, CocoaPods may not be present, causing “pod: command not found” or gem resolution drift. Use ruby/setup-ruby and bundle exec.

Apply this change outside this block to set up Ruby and gems (add after Xcode setup, before “Install iOS dependencies”):

- name: Set up Ruby
  uses: ruby/setup-ruby@v1
  with:
    ruby-version: '3.2'
    bundler-cache: true
  working-directory: app

- name: Install Ruby dependencies
  run: bundle install --jobs 4 --retry 3
  working-directory: app

Then minimally adjust this step to leverage bundler:

-          (cd app/ios && pod install --silent) || { echo "❌ Pod install failed"; exit 1; }
+          (cd app/ios && bundle exec pod install --silent) || { echo "❌ Pod install failed"; exit 1; }

329-336: Simulator creation uses invalid runtime identifier and ignores detected runtime

Using "iOS17.5" is not a valid runtime identifier (expected “com.apple.CoreSimulator.SimRuntime.iOS-17-5”), and the earlier detected runtime is not used. This will cause sim creation to fail on many runners.

Apply this diff:

-            echo "Creating a new iPhone SE (3rd generation) simulator..."
-            # Create a new iPhone SE (3rd generation) simulator with the latest iOS version
-            xcrun simctl create "iPhone SE (3rd generation)" "iPhone SE (3rd generation)" "iOS17.5" || {
+            echo "Creating a new iPhone SE (3rd generation) simulator..."
+            # Create a new simulator using the latest available iOS runtime
+            RUNTIME_ID="${RUNTIME_ID:-$(xcrun simctl list -j runtimes | jq -r '.runtimes[] | select(.platform=="iOS" and .isAvailable==true) | .identifier' | sort -V | tail -1)}"
+            if [ -z "$RUNTIME_ID" ]; then
+              echo "❌ Could not determine an available iOS runtime identifier"
+              exit 1
+            fi
+            xcrun simctl create "iPhone SE (3rd generation)" "iPhone SE (3rd generation)" "$RUNTIME_ID" || {
               echo "❌ Failed to create iPhone SE (3rd generation) simulator"
               echo "Trying to create any iPhone SE simulator..."
-              xcrun simctl create "iPhone SE" "iPhone SE" "iOS17.5" || {
+              xcrun simctl create "iPhone SE" "iPhone SE" "$RUNTIME_ID" || {
                 echo "❌ Failed to create simulator"
                 exit 1
               }
             }

252-260: Caching iOS Simulator directories is fragile and bloats caches

Caching CoreSimulator Devices and iOS DeviceSupport often corrupts simulators across runs and wastes space. Prefer not caching these.
[performance]

Apply this diff to remove the simulator cache:

-      - name: Cache iOS Simulator
-        uses: actions/cache@v4
-        with:
-          path: |
-            ~/Library/Developer/CoreSimulator/Devices
-            ~/Library/Developer/Xcode/iOS DeviceSupport
-          key: ${{ runner.os }}-simulator-v1
-          restore-keys: |
-            ${{ runner.os }}-simulator-

221-226: Ensure RUBY_VERSION Is Defined for the Bundler Cache Key

The Bundler cache step at lines 221–226 uses ${{ env.RUBY_VERSION }} but no RUBY_VERSION is set anywhere in this workflow. As a result, your cache key will always resolve to …-ruby (with an empty suffix), leading to ineffective cache differentiation and more cache misses.

Please add a concrete Ruby version to your environment or remove the Ruby component from the key. For example, to pin Ruby 3.2 globally:

• File: .github/workflows/mobile-e2e.yml
Location: top-level env: block (around lines 1–30)

Suggested diff:

 env:
   # Build environment versions
   JAVA_VERSION: 17
   ANDROID_API_LEVEL: 33
   ANDROID_NDK_VERSION: 27.0.11718014
   # Cache versions
   GH_CACHE_VERSION: v1 # Global cache version
   GH_GEMS_CACHE_VERSION: v1 # Ruby gems cache version
+  RUBY_VERSION: 3.2          # Specify Ruby version for Bundler cache
   # Performance optimizations
   GRADLE_OPTS: -Dorg.gradle.daemon=false …

Alternatively, if you prefer not to pin Ruby here, remove -ruby${{ env.RUBY_VERSION }} from the cache-version key under the Bundler cache step.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 64c14ed and 58dcf66.

📒 Files selected for processing (2)
  • .github/workflows/mobile-ci.yml (5 hunks)
  • .github/workflows/mobile-e2e.yml (4 hunks)
⏰ 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: run_circuit_tests

@transphorm transphorm force-pushed the codex/find-opportunities-to-speed-up-ios-e2e-tests branch from bfc871e to 6959ffb Compare August 19, 2025 00:52
@transphorm transphorm changed the title chore: streamline ios e2e workflow chore: fix ios e2e workflow; upgrade to Xcode 16.4 Aug 19, 2025
@transphorm transphorm requested a review from remicolin August 19, 2025 10:59
fi
echo "📱 Verifying iOS Runtime availability..."
echo "Available iOS runtimes:"
xcrun simctl list runtimes | grep iOS || true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why || true?

Copy link
Member Author

@transphorm transphorm Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

old leftover from previous logic that would go through a series of checks for the correct runtime - https://github.com/selfxyz/self/blob/dev/.github/workflows/mobile-e2e.yml#L260

the logic was mainly for testing the pipeline locally using act but we can just use github workflows for now.

it prevents a command run failure if no iOS runtimes are found. previously we should manually select a runtime.

will remove || true but shouldn't be a blocker

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants