-
Notifications
You must be signed in to change notification settings - Fork 1.4k
chore: improve ios build on CI #6748
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughWraps the iOS build step in a retry action (3 attempts, 60s wait, 60-minute timeout), adds 14-day retention to iOS dSYM and Info.plist artifact uploads, adds a top-level concurrency policy to the develop workflow, and removes explicit 60-minute timeouts from iOS build workflows. (34 words) Changes
Sequence Diagram(s)sequenceDiagram
participant DevWF as build-develop.yml
participant Runner as CI Runner
participant Action as build-ios/action.yml
participant Retry as nick-fields/retry@v3
DevWF->>Runner: start job (concurrency may cancel previous)
Runner->>Action: invoke build-ios action
Action->>Retry: execute build command in `ios/` (max_attempts=3, wait=60s, timeout=60m)
alt build succeeds within attempts
Retry->>Runner: build success
Runner->>Action: upload artifacts (dSYM, Info.plist) with retention-days: 14
else attempts exhausted / failure
Retry->>Runner: final failure
Runner->>Action: upload any produced artifacts (if present) with retention-days: 14
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 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 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
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 |
|
iOS Build Available Rocket.Chat Experimental 4.66.0.107610 |
There was a problem hiding this 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
🧹 Nitpick comments (1)
.github/workflows/build-develop.yml (1)
11-13: Concurrency control correctly configured; group naming is slightly verbose.The concurrency block correctly prevents overlapping runs and cancels outdated jobs. The group name
${{ github.workflow }}-developresolves to "Build Develop-develop" (redundant "develop" suffix), though this is harmless and still functional. If clarity is desired, consider simplifying to${{ github.workflow }}or a custom identifier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/actions/build-ios/action.yml(2 hunks).github/workflows/build-develop.yml(1 hunks)
🔇 Additional comments (3)
.github/actions/build-ios/action.yml (3)
117-130: Retry wrapper correctly implemented; verify timeout semantics.The nick-fields/retry@v3 wrapper with 3 attempts and 60-second retry delay is a sound approach to mitigate transient macOS CI failures. Environment variables are correctly positioned outside the action, ensuring proper scoping across retries.
However, clarify the timeout behavior: Does
timeout_minutes: 30apply per attempt (potentially up to ~100 minutes total with waits) or across all retries? Verify this aligns with your CI capacity and expected build duration.
117-130: Verify exponential backoff claim from AI summary.The AI summary states the build uses "exponential backoff," but the visible configuration shows
retry_wait_seconds: 60(fixed delay). Verify whether:
- nick-fields/retry@v3 applies exponential backoff automatically despite the fixed value, or
- The summary overstates the retry behavior (actual behavior is fixed 60-second delays).
153-183: Artifact retention policy appropriately applied to debug and metadata artifacts.The 14-day retention period for dSYM and Info.plist artifacts is a sensible choice for post-build debugging and compliance. IPA artifacts are intentionally excluded, likely due to storage constraints or policy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.github/actions/build-ios/action.yml(2 hunks).github/workflows/build-ios.yml(0 hunks).github/workflows/build-official-ios.yml(0 hunks)
💤 Files with no reviewable changes (2)
- .github/workflows/build-official-ios.yml
- .github/workflows/build-ios.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🔇 Additional comments (4)
.github/actions/build-ios/action.yml (4)
116-131: Verify timeout reduction from 60 to 50 minutes is safe.The retry step wraps the iOS build with 3 retries and a 50-minute timeout. However, the AI summary indicates the previous timeout was 60 minutes. If typical builds regularly consume 55+ minutes, the new 50-minute limit could cause legitimate builds to timeout prematurely, triggering unnecessary retries and slowing CI.
Also note:
retry_on: timeoutmeans only timeout failures retry; build errors or dependency issues fail immediately without retry. Confirm this aggressive behavior is intentional.Please verify:
- Typical build duration: Does the iOS build normally take 50–60 minutes? If so, the timeout may be too tight.
- Intended retry behavior: Should the retry action only retry on timeout, or should it also retry on other transient failures?
You can check recent CI run logs to estimate typical build duration.
160-160: Artifact retention policy looks good.Adding 14-day explicit retention to dSYM and Info.plist artifacts is a best practice for debugging and compliance. The configuration is consistent across both official and experimental build paths, and 14 days is a reasonable balance between retention needs and storage costs.
Also applies to: 168-168, 176-176, 184-184
116-131: Verify timeout reduction from 60 to 50 minutes is safe for your typical build duration.The retry step wraps the iOS build with 3 retries and a 50-minute timeout. The AI summary indicates the previous timeout was 60 minutes. If your builds regularly take 55–60 minutes, the new 50-minute limit could cause legitimate builds to timeout prematurely, triggering unnecessary retries and wasting CI resources.
Also note:
retry_on: timeoutmeans only timeout failures will retry; build errors or transient dependency issues fail immediately without retry. Confirm this fail-fast behavior is intentional.Please verify:
- Typical build duration: Review recent CI run logs to confirm whether iOS builds normally take 50–55 minutes. If they frequently approach 55–60 minutes, the new 50-minute timeout may be too tight.
- Retry strategy: Confirm that retrying only on timeout (not on all transient failures) is the intended behavior.
160-160: Artifact retention configuration is sound.Adding explicit 14-day retention to dSYM and Info.plist artifacts is a best practice for debugging and compliance. The retention is applied consistently across both official and experimental build paths, and 14 days is a reasonable balance between retention needs and storage costs.
Also applies to: 168-168, 176-176, 184-184
| if [[ ${{ inputs.type }} == "experimental" ]]; then | ||
| bundle exec fastlane ios build_experimental | ||
| fi | ||
| uses: nick-fields/retry@v3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify nick-fields/retry@v3 is actively maintained.
The retry wrapper improves CI stability for transient macOS runner hangs (per PR objectives). However, nick-fields/retry is a third-party community action, so verify it is actively maintained and free from known security issues.
🌐 Web query:
nick-fields/retry v3 GitHub action maintenance security vulnerabilities
💡 Result:
Short answer: nick-fields/retry@v3 is widely used but appears effectively unmaintained and has flagged security/maintenance issues; security scanners recommend replacing or pinning it. [1][2][3]
Details and sources
- Official repo/Marketplace listing (nick-fields/retry v3) — describes the action and ownership but shows repository issues/PRs; no guarantee of active maintenance. [1]
- Security/quality scan (StepSecurity / Action Advisor) flags nick-fields/retry with low score, multiple detected vulnerabilities, and little recent maintenance activity; it recommends the drop-in alternative step-security/retry. [2]
- Independent coverage of GitHub Actions supply‑chain risks explains why using mutable tags (e.g., @V3) or unmaintained third‑party actions increases risk. Best practice: pin to a commit SHA or use a maintained, audited replacement. [3]
Recommendations
- Replace nick-fields/retry@v3 with a maintained alternative (e.g., step-security/retry) or another audited action. [2]
- If you must keep using it, pin to a specific commit SHA (not the mutable @V3 tag) and review the action code; monitor for CVEs and upstream fixes. [3]
Sources
[1] GitHub Marketplace / nick-fields/retry (action docs).
[2] StepSecurity / Action Advisor scan for retry (security score, vulnerabilities, recommendation).
[3] Analysis of GitHub Actions supply‑chain risks and best practices.
Replace unmaintained dependency or pin to specific commit SHA.
nick-fields/retry@v3 is unmaintained and flagged with multiple security vulnerabilities and low maintenance activity by security scanners. Use a maintained alternative like step-security/retry, or if you must retain nick-fields/retry, pin to a specific commit SHA rather than the mutable @V3 tag.
🤖 Prompt for AI Agents
In .github/actions/build-ios/action.yml around line 117, the workflow uses the
unmaintained action reference "nick-fields/retry@v3"; update this to either a
maintained alternative (for example "step-security/retry@<latest-tag-or-sha>" or
another actively maintained retry action) or pin the existing action to an
immutable commit SHA (e.g., "nick-fields/retry@<commit-sha>") to avoid mutable
tags; change the uses line accordingly and run a quick CI to validate behavior.
🧩 Analysis chain
Verify nick-fields/retry@v3 is actively maintained.
The retry action improves stability for transient macOS hangs (per PR objectives), but it's a third-party action. Confirm it's actively maintained and free from known vulnerabilities.
🌐 Web query:
nick-fields/retry@v3 GitHub action maintenance status security
💡 Result:
Short answer
- nick-fields/retry@v3 is a third‑party GitHub Action still published (Marketplace README shows v3 and author/ownership). [1]
- Multiple automated security scanners flag the action as low-maintenance and having unresolved findings; StepSecurity’s Action Advisor reports no commit/issue activity in the last 90 days and several detected vulnerabilities, and recommends step-security/retry as a maintained drop‑in replacement. [2][3]
- Public telemetry (issue/PR counts) shows substantial past use but many open issues/PRs on the repo, indicating limited recent maintenance. [4]
What I recommend
- If you require a maintained/secure action, replace nick-fields/retry@v3 with a vetted maintained alternative (e.g., step-security/retry) or pin to a commit and review its code & dependencies before use. [2]
- If you must keep nick-fields/retry, audit its dependencies, enable least-privilege tokens, and rotate any secrets if you suspect exposure.
Sources
- Marketplace README for retry (nick-fields). [GitHub Marketplace]. [1]
- StepSecurity Action Advisor security/maintenance comparison (recommends step-security/retry). [2]
- StepSecurity blog/coverage and wider reporting on GitHub Actions supply‑chain issues (context for risk). [5][6]
- Public repo/issue stats for nick-fields (ecosyste.ms summary showing issue/PR activity). [4]
Would you like me to (a) check the repo’s most recent commit date and open-issue timestamps now, or (b) prepare a one-line workflow replacement using step-security/retry?
Replace nick-fields/retry@v3 with an actively maintained alternative.
StepSecurity Action Advisor reports no commit/issue activity in the last 90 days and several detected vulnerabilities. step-security/retry is recommended as a maintained drop-in replacement. Switch to that action or audit dependencies and enable least-privilege tokens if you must retain this action.
🤖 Prompt for AI Agents
.github/actions/build-ios/action.yml around line 117: the workflow uses an
unmaintained action nick-fields/retry@v3; replace it with the actively
maintained drop-in alternative step-security/retry (use the latest stable tag,
e.g. step-security/retry@v1) and verify input/option names match the original
action; after swapping, run the workflow to confirm behavior is unchanged and,
if you must keep any third‑party action, audit its dependencies and tighten
workflow permissions (least-privilege tokens) in the workflow file.
|
iOS Build Available Rocket.Chat Experimental 4.67.0.107646 |
|
iOS Build Available Rocket.Chat Experimental 4.68.0.107787 |
Proposed changes
The goal of this PR
retry-stepaction to the iOS build workflow to improve stability (macOS runners often hang,retry-stepalready improved Maestro builds by ~95%)Action Link: https://github.com/RocketChat/Rocket.Chat.ReactNative/actions/runs/18877586230?pr=6748
Issue(s)
https://rocketchat.atlassian.net/browse/COMM-58
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit