-
Notifications
You must be signed in to change notification settings - Fork 180
Update main 10/10/25
#1259
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
Update main 10/10/25
#1259
Conversation
* add only triggers * tweak release * formatting
…nimations-fix release: iOS bugfix build v2.6.9
) * chore: bump iOS build number to 179 * fix: use PR source commit for deployment to get correct version.json When deploying from a PR merge (e.g., dev → staging), now uses the source branch's commit instead of always checking out staging. This ensures version.json has the correct bumped build number from the previous deployment's PR back to dev, preventing 'build number already exists' errors. * downgrade to match store * fix: use merge_commit_sha instead of head.sha for deployments Use github.event.pull_request.merge_commit_sha instead of head.sha to ensure we deploy exactly what landed on staging after the merge, not just the source branch state. This correctly handles: - Conflict resolutions made during merge - Any staging-only changes - The actual state of staging post-merge The merge commit still includes the updated version.json from the source branch (e.g., dev), so build numbers remain correct while ensuring we deploy and tag the true staging state. Co-authored-by: CodeRabbit <[email protected]> --------- Co-authored-by: CodeRabbit <[email protected]>
chore: fix mobile auto deploy v2.6.9 rd2
…version0v269 fix build version for v2.6.9
chore: bump iOS build for 2.6.9
* refine mobile deploy auto pr logic * remove env check * fix get-version
…1244) * bump version to match staging * save wip * deploy fixes * fix version setting * update version logic * fix version pr * increase timeout to 2 hours * pr logic tweaks * fix script * fix script path * add comments and update logic to test from feature branch * fix build path * fix version input error * fix pulling version * add skip-deploy lable * address cr concners
staging: test mobile deploy auto pull request (#1234)
…-to-staging staging: update release calendar logic (#1256)
WalkthroughAdds CI-driven version management: new composite action to create version-bump PRs, enhanced get-version action, centralized Node.js version-manager CLI, Fastlane changes to verify CI-provided versions, multiple GitHub workflow additions/refactors (mobile-deploy, release-calendar), app version bumps, and a DelayedLottieView plus replacements across screens. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer/Trigger
participant GH as GitHub Actions
participant VM as version-manager (script/action)
participant B as bump-version job
participant PR as create-version-bump-pr action
participant TAG as create-release-tags job
Dev->>GH: push / PR / dispatch / schedule
GH->>B: run bump-version (invoke VM get/bump/apply)
B-->>GH: outputs(version, ios_build, android_build, platform, version_bump_type)
GH->>PR: create/update version-bump branch & PR using GH_TOKEN
PR-->>GH: PR created/labelled (idempotent)
GH->>TAG: tag & release creation using bump outputs
TAG-->>Dev: tags/releases published
note right of VM: Centralized version operations (get/bump/apply)
sequenceDiagram
autonumber
participant Screen as App Screen
participant DLV as DelayedLottieView
participant LV as LottieView (native)
Screen->>DLV: mount(autoPlay=true, source, loop)
DLV->>LV: render(props with autoPlay=false)
DLV->>DLV: setTimeout(100ms)
DLV->>LV: play()
note right of DLV: Defers start for iOS native module init
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ 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 |
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: 6
🧹 Nitpick comments (7)
app/src/components/DelayedLottieView.tsx (1)
28-37: Consider removingreffrom the useEffect dependency array.The
refobject is stable after the first render (eitherforwardedReforinternalRef). Including it in the dependency array is unnecessary and could cause confusion. The effect only needs to re-run whenprops.autoPlaychanges.Apply this diff:
- }, [props.autoPlay, ref]); + }, [props.autoPlay]);app/src/screens/document/DocumentOnboardingScreen.tsx (1)
36-43: Consider using DelayedLottieView for consistency.While the manual delay implementation works correctly, other screens in this PR use
DelayedLottieView. SinceDelayedLottieViewonly auto-triggers whenautoPlay={true}, you could use it here withautoPlay={false}and still manually control playback via the ref, maintaining consistency across the codebase.Current approach:
// Manual delay in useEffect useEffect(() => { const timer = setTimeout(() => { animationRef.current?.play(); }, 100); return () => clearTimeout(timer); }, []);Alternative with DelayedLottieView:
+import { DelayedLottieView } from '@/components/DelayedLottieView'; -import LottieView from 'lottie-react-native'; -<LottieView +<DelayedLottieView ref={animationRef} autoPlay={false} ... />Then the same manual
play()call would work, but you'd have a consistent component across all screens..github/workflows/release-calendar.yml (2)
147-158: Handle potential branch name collisions.The branch creation steps could fail if a release branch already exists from a previous workflow run that didn't complete or from manual intervention.
Consider adding branch existence checks or force-pushing:
- name: Create release branch from dev if: ${{ steps.guard_schedule.outputs.continue == 'true' && steps.check_dev_staging.outputs.existing_pr == '' }} env: BRANCH_NAME: ${{ steps.check_dev_staging.outputs.branch_name }} shell: bash run: | set -euo pipefail echo "Creating release branch ${BRANCH_NAME} from dev" git fetch origin dev - git checkout -b "${BRANCH_NAME}" origin/dev - git push origin "${BRANCH_NAME}" + # Delete local branch if it exists + git branch -D "${BRANCH_NAME}" 2>/dev/null || true + git checkout -b "${BRANCH_NAME}" origin/dev + # Force push in case remote branch exists from failed previous run + git push --force origin "${BRANCH_NAME}"Apply similar changes to the production branch creation step (lines 330-341).
Also applies to: 330-341
170-205: PR body generation is functional, consider extraction if complexity grows.The embedded Python scripts work well for the current use case. If you find yourself adding more logic or reusing this pattern, consider extracting to dedicated template files for easier maintenance.
Also applies to: 354-392
.github/actions/create-version-bump-pr/action.yml (1)
29-46: Add error handling for git operations.The git commands (fetch, checkout, push) can fail but lack explicit error handling. While bash's default behavior will exit on error in most cases, explicitly checking critical operations improves reliability and debugging.
Consider adding error checks after critical operations:
git fetch origin staging dev + if [ $? -ne 0 ]; then + echo "❌ Failed to fetch branches" + exit 1 + fi git checkout staging + if [ $? -ne 0 ]; then + echo "❌ Failed to checkout staging" + exit 1 + fiOr use
set -eat the start of the script to exit on any command failure.app/scripts/version-manager.cjs (1)
206-224: Add version format validation in applyVersions.The
applyVersionsfunction accepts a version string without validating its format. This could allow invalid versions to be written topackage.json, potentially breaking downstream tools that expect semantic versioning.Apply this diff to add validation:
function applyVersions(version, iosBuild, androidBuild) { + // Validate version format + const versionParts = version.split('.').map(Number); + if (versionParts.length !== 3 || versionParts.some(isNaN)) { + throw new Error( + `Invalid version format: ${version}. Expected X.Y.Z (semantic versioning)`, + ); + } + console.log(`📝 Applying versions to files...`); console.log(` Version: ${version}`);.github/workflows/mobile-deploy.yml (1)
1260-1277: Consider using version-manager.cjs instead of inline Node.js.The inline Node.js scripts duplicate logic that already exists in
app/scripts/version-manager.cjs. Using the centralized script would improve maintainability and reduce duplication.Apply this diff to use the version-manager script:
- name: Apply version bump from outputs run: | cd ${{ env.APP_PATH }} VERSION="${{ needs.bump-version.outputs.version }}" IOS_BUILD="${{ needs.bump-version.outputs.ios_build }}" ANDROID_BUILD="${{ needs.bump-version.outputs.android_build }}" echo "📝 Applying version bump: $VERSION (iOS: $IOS_BUILD, Android: $ANDROID_BUILD)" - # Update package.json version - node -e " - const fs = require('fs'); - const pkg = JSON.parse(fs.readFileSync('package.json', 'utf8')); - pkg.version = '$VERSION'; - fs.writeFileSync('package.json', JSON.stringify(pkg, null, 2) + '\n'); - console.log('✅ Updated package.json'); - " - - # Update version.json build numbers - node -e " - const fs = require('fs'); - const version = JSON.parse(fs.readFileSync('version.json', 'utf8')); - version.ios.build = $IOS_BUILD; - version.android.build = $ANDROID_BUILD; - fs.writeFileSync('version.json', JSON.stringify(version, null, 2) + '\n'); - console.log('✅ Updated version.json'); - " + # Use version-manager script to apply versions + node scripts/version-manager.cjs apply "$VERSION" "$IOS_BUILD" "$ANDROID_BUILD" echo "✅ Versions applied successfully"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
.github/actions/create-version-bump-pr/action.yml(1 hunks).github/actions/get-version/action.yml(1 hunks).github/workflows/mobile-deploy.yml(18 hunks).github/workflows/mobile-e2e.yml(2 hunks).github/workflows/release-calendar.yml(1 hunks)app/android/app/build.gradle(1 hunks)app/fastlane/Fastfile(6 hunks)app/fastlane/helpers/version_manager.rb(1 hunks)app/ios/OpenPassport/Info.plist(1 hunks)app/ios/Self.xcodeproj/project.pbxproj(2 hunks)app/package.json(1 hunks)app/scripts/version-manager.cjs(1 hunks)app/src/components/DelayedLottieView.tsx(1 hunks)app/src/components/loading/LoadingUI.tsx(2 hunks)app/src/screens/document/DocumentCameraScreen.tsx(2 hunks)app/src/screens/document/DocumentOnboardingScreen.tsx(1 hunks)app/src/screens/home/DisclaimerScreen.tsx(2 hunks)app/src/screens/prove/ConfirmBelongingScreen.tsx(2 hunks)app/src/screens/recovery/AccountVerifiedSuccessScreen.tsx(2 hunks)app/src/screens/system/SplashScreen.tsx(2 hunks)app/version.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
app/src/screens/document/DocumentCameraScreen.tsxapp/src/screens/prove/ConfirmBelongingScreen.tsxapp/src/components/DelayedLottieView.tsxapp/src/screens/system/SplashScreen.tsxapp/src/screens/home/DisclaimerScreen.tsxapp/src/screens/recovery/AccountVerifiedSuccessScreen.tsxapp/src/components/loading/LoadingUI.tsxapp/src/screens/document/DocumentOnboardingScreen.tsx
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Type checking must pass before PRs (yarn types)
Files:
app/src/screens/document/DocumentCameraScreen.tsxapp/src/screens/prove/ConfirmBelongingScreen.tsxapp/src/components/DelayedLottieView.tsxapp/src/screens/system/SplashScreen.tsxapp/src/screens/home/DisclaimerScreen.tsxapp/src/screens/recovery/AccountVerifiedSuccessScreen.tsxapp/src/components/loading/LoadingUI.tsxapp/src/screens/document/DocumentOnboardingScreen.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/screens/document/DocumentCameraScreen.tsxapp/src/screens/prove/ConfirmBelongingScreen.tsxapp/src/components/DelayedLottieView.tsxapp/src/screens/system/SplashScreen.tsxapp/src/screens/home/DisclaimerScreen.tsxapp/src/screens/recovery/AccountVerifiedSuccessScreen.tsxapp/src/components/loading/LoadingUI.tsxapp/src/screens/document/DocumentOnboardingScreen.tsx
app/android/**/*
⚙️ CodeRabbit configuration file
app/android/**/*: Review Android-specific code for:
- Platform-specific implementations
- Performance considerations
- Security best practices for mobile
Files:
app/android/app/build.gradle
.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-e2e.yml.github/workflows/release-calendar.yml.github/workflows/mobile-deploy.yml
app/ios/**/*
⚙️ CodeRabbit configuration file
app/ios/**/*: Review iOS-specific code for:
- Platform-specific implementations
- Performance considerations
- Security best practices for mobile
Files:
app/ios/OpenPassport/Info.plistapp/ios/Self.xcodeproj/project.pbxproj
app/package.json
📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)
Expose a 'test:build' script in the app's package.json that builds deps, types, performs bundle analysis, and runs tests
Files:
app/package.json
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use Yarn only for package management (yarn install/add/remove); do not use npm or pnpm in scripts
Files:
app/package.json
🧠 Learnings (2)
📚 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-deploy.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-bundler composite action for Ruby gems caching in workflows
Applied to files:
.github/workflows/mobile-deploy.yml
🧬 Code graph analysis (6)
app/src/screens/document/DocumentCameraScreen.tsx (1)
app/src/components/DelayedLottieView.tsx (1)
DelayedLottieView(23-46)
app/src/screens/prove/ConfirmBelongingScreen.tsx (1)
app/src/components/DelayedLottieView.tsx (1)
DelayedLottieView(23-46)
app/src/screens/system/SplashScreen.tsx (1)
app/src/components/DelayedLottieView.tsx (1)
DelayedLottieView(23-46)
app/src/screens/home/DisclaimerScreen.tsx (1)
app/src/components/DelayedLottieView.tsx (1)
DelayedLottieView(23-46)
app/src/screens/recovery/AccountVerifiedSuccessScreen.tsx (1)
app/src/components/DelayedLottieView.tsx (1)
DelayedLottieView(23-46)
app/src/components/loading/LoadingUI.tsx (1)
app/src/components/DelayedLottieView.tsx (1)
DelayedLottieView(23-46)
🪛 GitHub Check: workspace-lint
app/src/components/DelayedLottieView.tsx
[warning] 23-23:
Caution: React also has a named export forwardRef. Check if you meant to write import {forwardRef} from 'react' instead
🪛 RuboCop (1.81.1)
app/fastlane/helpers/version_manager.rb
[convention] 47-78: Assignment Branch Condition size for verify_ci_version_match is too high. [<9, 20, 9> 23.71/23]
(Metrics/AbcSize)
app/fastlane/Fastfile
[warning] 136-136: Useless assignment to variable - version_bump.
(Lint/UselessAssignment)
[warning] 307-307: Useless assignment to variable - version_bump. Did you mean version_code?
(Lint/UselessAssignment)
⏰ 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: quality-checks
- GitHub Check: build-deps
- GitHub Check: android-build-test
- GitHub Check: e2e-ios
- GitHub Check: analyze-android
- GitHub Check: analyze-ios
🔇 Additional comments (21)
app/ios/OpenPassport/Info.plist (1)
24-24: LGTM! Version bump aligns with release.The CFBundleShortVersionString update from 2.6.8 to 2.6.9 is consistent with the coordinated version bump across iOS and Android platforms.
app/android/app/build.gradle (1)
132-133: Verify the versionCode jump from 102 to 108.The versionName bump from 2.6.8 to 2.6.9 is standard, but the versionCode jumped from 102 to 108 (skipping 6 versions). Ensure this aligns with your release process—typically versionCode increments by 1 per build.
Were versionCodes 103-107 used for internal/beta builds, or is this gap intentional?
.github/workflows/mobile-e2e.yml (1)
37-37: Timeout increase is reasonable for E2E stability.Doubling the timeout from 60 to 120 minutes for both Android and iOS E2E jobs should prevent timeouts on slower CI runners or when tests expand. Consider monitoring actual test durations to identify optimization opportunities if tests consistently approach this new limit.
Also applies to: 181-181
app/ios/Self.xcodeproj/project.pbxproj (1)
545-545: LGTM! Xcode marketing version updated consistently.The MARKETING_VERSION is correctly set to 2.6.9 in both Debug and Release configurations, aligning with the Info.plist update.
Also applies to: 685-685
app/src/components/DelayedLottieView.tsx (1)
23-46: LGTM! Solid implementation for fixing iOS animation timing.The DelayedLottieView wrapper correctly addresses iOS native module initialization timing by:
- Disabling native autoPlay and controlling it manually
- Adding a 100ms delay before triggering animations
- Properly forwarding refs and cleaning up timers
The static analysis warning about using a named import for
forwardRefis a style preference—React.forwardRefis perfectly valid.app/src/screens/home/DisclaimerScreen.tsx (1)
14-14: LGTM! Clean migration to DelayedLottieView.The switch from LottieView to DelayedLottieView is implemented correctly, maintaining all existing props while fixing the iOS initialization timing issue.
Also applies to: 33-40
app/src/components/loading/LoadingUI.tsx (1)
5-5: LGTM! Elegant type-only import pattern.The migration to DelayedLottieView uses a type-only import for LottieView (line 5), keeping it for type references while using DelayedLottieView at runtime. This is a clean pattern that maintains type safety while fixing the iOS initialization issue.
Also applies to: 10-10, 121-133
app/src/screens/prove/ConfirmBelongingScreen.tsx (1)
21-21: LGTM! Consistent DelayedLottieView migration.The migration follows the same pattern as other screens, correctly replacing LottieView with DelayedLottieView to address iOS native module initialization timing.
Also applies to: 89-96
app/src/screens/recovery/AccountVerifiedSuccessScreen.tsx (1)
13-13: LGTM! Consistent migration to DelayedLottieView.The replacement preserves all animation properties while addressing iOS initialization timing through the centralized delay mechanism.
Also applies to: 27-34
app/src/screens/system/SplashScreen.tsx (1)
16-16: LGTM! Migration preserves critical navigation flow.The
onAnimationFinishcallback is correctly passed through and will trigger after the delayed animation completes, maintaining the existing navigation behavior.Also applies to: 119-128
app/src/screens/document/DocumentCameraScreen.tsx (1)
22-22: LGTM! Standard migration pattern applied correctly.Also applies to: 61-68
app/package.json (1)
3-3: LGTM! Version bump aligns with platform-specific version updates.The patch version increment is consistent with the changes in iOS and Android build files.
app/version.json (1)
3-4: LGTM! Build metadata updated correctly.Build number increments and deployment timestamps reflect the platform-specific release cadence.
Also applies to: 7-8
.github/actions/get-version/action.yml (1)
10-13: LGTM! Improves action reusability.Exposing the version as an output enables better workflow composition and makes this action more versatile for downstream jobs.
Also applies to: 19-19, 23-23
.github/workflows/release-calendar.yml (1)
55-94: LGTM! Guard logic correctly handles all trigger scenarios.The conditional execution based on event type and day-of-week prevents unwanted PR creation while supporting both automated scheduling and manual overrides.
Also applies to: 223-257
app/scripts/version-manager.cjs (1)
1-329: LGTM! Well-structured version manager.The centralized version manager provides a clean API for version operations and follows good practices:
- Clear separation of concerns (read/write/bump/apply)
- Both CLI and programmatic interfaces
- Good error handling with descriptive messages
- Proper file I/O with error catching
- Platform-specific build number logic
The script will serve well as the single source of truth for versioning across GitHub Actions, Fastlane, and local development.
.github/workflows/mobile-deploy.yml (2)
149-243: LGTM! Excellent atomic version bump design.The new
bump-versionjob calculates versions once before platform builds, preventing race conditions. Key benefits:
- Single source of truth for version numbers via outputs
- Platform and bump type determined from PR labels or inputs
- Clean separation: bump calculation → platform builds → PR creation
- Proper checkout of triggering branch (staging for PRs, feature branch for manual)
The centralized approach with version-manager.cjs and idempotent PR creation is well-architected.
705-735: Good addition of build artifact verification.The iOS build verification step (and matching Android verification on lines 1150-1180) adds important safety checks:
- Confirms IPA/AAB files exist after Fastlane completes
- Validates file size to catch silent build failures
- Provides clear error messages for debugging
This helps catch issues that might otherwise go unnoticed until deployment.
app/fastlane/Fastfile (3)
98-102: Confirm CI check gating — consider verifying on all CI runsThe version verification runs only when version_bump == "skip". If CI remains the source of truth even when bumping, mismatches can slip by. Consider running verification for all CI runs (or at least logging mismatches) to harden the pipeline.
If intended to verify unconditionally in CI, update as:
- if is_ci && version_bump == "skip" + if is_ci Fastlane::Helpers.verify_ci_version_match endAlso applies to: 291-295
175-178: LGTM: iOS build number sourced from version.jsonUsing version.json as the single source ensures parity with CI-managed values; the log aids traceability.
332-335: LGTM: Android versionCode sourced from version.jsonConsistent with CI-managed versioning; clear log line is helpful for debugging builds.
| file_paths: | ||
| description: File paths to include in the PR (newline separated) | ||
| required: true |
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.
Unused input parameter.
The file_paths input is declared but never referenced in the composite action's script. Consider either using it in the PR body or removing it if not needed.
If the intent is to include file paths in the PR body, apply this diff:
--title "$PR_TITLE" \
- --body "Automated version bump by CI" \
+ --body "Automated version bump by CI
+
+Files changed:
+${{ inputs.file_paths }}" \
--label "automated"Otherwise, remove the unused input declaration.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .github/actions/create-version-bump-pr/action.yml around lines 10 to 12 the
input "file_paths" is declared but never used; either remove this input
declaration or wire it into the composite action so it is consumed. To fix, if
you want to keep it, propagate inputs.file_paths into the action's steps (export
as an env var or pass as with: to the step that creates the PR) and include it
in the PR body or title (e.g., append the file_paths value to the body text
passed to the create-pull-request/gh CLI step); otherwise delete the file_paths
input entry from the action.yml to avoid an unused parameter.
| git push --set-upstream origin ${BRANCH_NAME} | ||
| # Determine PR title based on platform | ||
| if [ "${{ inputs.platform }}" = "mobile" ]; then |
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.
Invalid platform value in conditional.
The conditional checks for platform value "mobile", but the input description (line 5) states the platform should be "ios or android". This will never match, causing the PR title logic to always fall through to the else branch.
Apply this diff to fix the logic:
- if [ "${{ inputs.platform }}" = "mobile" ]; then
+ if [ "${{ inputs.platform }}" = "both" ]; then
PR_TITLE="chore: bump mobile app version to ${{ inputs.version }}"
else
PR_TITLE="chore: bump ${{ inputs.platform }} build for ${{ inputs.version }}"Or consider removing the conditional entirely if both platforms always create the same PR title format.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
.github/actions/create-version-bump-pr/action.yml around line 49: the
conditional currently compares inputs.platform to "mobile" which never matches
because the input is documented as "ios or android"; update the conditional to
check for the actual values (e.g., if [ "${{ inputs.platform }}" = "ios" ] || [
"${{ inputs.platform }}" = "android" ]; then) so the PR title branch runs for
mobile platforms, or remove the conditional entirely if both branches should
produce the same PR title.
| dry_run: | ||
| description: "Do not commit/push or create PR/tags" | ||
| required: false | ||
| type: boolean | ||
| default: false | ||
| bump_target_branch: | ||
| description: "Target branch for version bump PR (default: dev). NOTE: This is where the version bump PR will be created, NOT the branch to build from. The workflow always builds from the triggering branch." | ||
| required: false | ||
| type: string | ||
| default: "dev" |
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 dry_run implementation is complete.
The dry_run input is declared but only used in one place (line 1317 in create-version-bump-pr). Ensure this covers all push/create operations, or consider if additional guards are needed for git push operations in the create-release-tags job.
Run the following to check all git push operations:
🏁 Script executed:
#!/bin/bash
# Find all git push commands in the workflow
rg -n "git push" .github/workflows/mobile-deploy.ymlLength of output: 167
Guard all git push operations with dry_run.
Wrap the pushes/tags in the create-release-tags job (lines 1353, 1454, 1458) behind if: ${{ !inputs.dry_run }} to block any actual push or tag creation when dry_run: true.
🤖 Prompt for AI Agents
.github/workflows/mobile-deploy.yml lines 98-107: the workflow introduces a
dry_run input but the create-release-tags job still performs git pushes and tag
creation unconditionally; wrap each step that performs push or tag creation in
the create-release-tags job (specifically the steps at ~lines 1353, 1454, 1458)
with an if: ${{ !inputs.dry_run }} so those steps are skipped when dry_run is
true, ensuring no commits/pushes or tag creation occur during dry runs.
| if [ -d "${{ env.APP_PATH }}/vendor/bundle" ]; then | ||
| GEMS_SIZE=$(du -sh "${{ env.APP_PATH }}/vendor/bundle" | cut -f1) | ||
| echo "Ruby gems: $GEMS_SIZE" |
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.
Incorrect Ruby gems cache path for iOS builds.
The workflow checks vendor/bundle but iOS builds should cache gems at app/ios/vendor/bundle according to the project's learnings and the cache configuration on line 359.
Based on learnings.
Apply this diff to use the correct path:
- if [ -d "${{ env.APP_PATH }}/vendor/bundle" ]; then
- GEMS_SIZE=$(du -sh "${{ env.APP_PATH }}/vendor/bundle" | cut -f1)
+ if [ -d "${{ env.APP_PATH }}/ios/vendor/bundle" ]; then
+ GEMS_SIZE=$(du -sh "${{ env.APP_PATH }}/ios/vendor/bundle" | cut -f1)
echo "Ruby gems: $GEMS_SIZE"
fi📝 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.
| if [ -d "${{ env.APP_PATH }}/vendor/bundle" ]; then | |
| GEMS_SIZE=$(du -sh "${{ env.APP_PATH }}/vendor/bundle" | cut -f1) | |
| echo "Ruby gems: $GEMS_SIZE" | |
| if [ -d "${{ env.APP_PATH }}/ios/vendor/bundle" ]; then | |
| GEMS_SIZE=$(du -sh "${{ env.APP_PATH }}/ios/vendor/bundle" | cut -f1) | |
| echo "Ruby gems: $GEMS_SIZE" | |
| fi |
🤖 Prompt for AI Agents
.github/workflows/mobile-deploy.yml around lines 791 to 793: the workflow is
checking for Ruby gems at "${{ env.APP_PATH }}/vendor/bundle" but iOS builds
cache gems under "app/ios/vendor/bundle" per the cache config (line 359); change
the path checks and GEMS_SIZE calculation to reference "${{ env.APP_PATH
}}/app/ios/vendor/bundle" (and update the echo accordingly) so the workflow
reports and measures the correct iOS gems cache location.
| def verify_ci_version_match | ||
| # Verify that versions were pre-set by CI | ||
| unless ENV["CI_VERSION"] && ENV["CI_IOS_BUILD"] && ENV["CI_ANDROID_BUILD"] | ||
| UI.user_error!("CI must set CI_VERSION, CI_IOS_BUILD, and CI_ANDROID_BUILD environment variables") | ||
| end |
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 method exposure path to avoid NoMethodError in Fastfile
Fastfile calls Fastlane::Helpers.verify_ci_version_match, while this defines Fastlane::Helpers::VersionManager.verify_ci_version_match. Ensure the Helpers aggregator includes/forwards VersionManager so the method resolves at runtime.
Run to verify inclusion/forwarding:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching where VersionManager is included/forwarded into Fastlane::Helpers..."
rg -n -C2 -g '!**/node_modules/**' -g '!**/build/**' 'module\s+Fastlane|module\s+Helpers|include\s+VersionManager|extend\s+VersionManager|verify_ci_version_match' appLength of output: 3472
Expose VersionManager in Fastlane::Helpers
Add extend Fastlane::Helpers::VersionManager to module Fastlane::Helpers in app/fastlane/helpers.rb so verify_ci_version_match is available at runtime.
🧰 Tools
🪛 RuboCop (1.81.1)
[convention] 47-78: Assignment Branch Condition size for verify_ci_version_match is too high. [<9, 20, 9> 23.71/23]
(Metrics/AbcSize)
🤖 Prompt for AI Agents
In app/fastlane/helpers.rb around the module Fastlane::Helpers declaration,
expose the VersionManager by adding extend Fastlane::Helpers::VersionManager
inside the module body; open the file, locate "module Fastlane::Helpers" and add
that single extend line (ensuring the VersionManager file is loaded beforehand
if not), then run a quick spec or load check to confirm verify_ci_version_match
is available as a module method at runtime.
| ios_build = get_ios_build_number | ||
| android_build = get_android_build_number | ||
|
|
||
| expected_version = ENV["CI_VERSION"] | ||
| expected_ios_build = ENV["CI_IOS_BUILD"].to_i | ||
| expected_android_build = ENV["CI_ANDROID_BUILD"].to_i | ||
|
|
||
| version_matches = pkg_version == expected_version | ||
| ios_matches = ios_build == expected_ios_build | ||
| android_matches = android_build == expected_android_build | ||
|
|
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.
Prevent false mismatches: coerce and validate build numbers as integers
Comparing JSON values (possibly strings) to ENV.to_i can misfire (e.g., "100" vs 100; nil.to_i => 0). Coerce both sides with Integer() to ensure type alignment and fail fast on invalid inputs.
Apply this diff:
- ios_build = get_ios_build_number
- android_build = get_android_build_number
+ ios_build = Integer(get_ios_build_number)
+ android_build = Integer(get_android_build_number)
@@
- expected_ios_build = ENV["CI_IOS_BUILD"].to_i
- expected_android_build = ENV["CI_ANDROID_BUILD"].to_i
+ expected_ios_build = Integer(ENV["CI_IOS_BUILD"])
+ expected_android_build = Integer(ENV["CI_ANDROID_BUILD"])📝 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.
| ios_build = get_ios_build_number | |
| android_build = get_android_build_number | |
| expected_version = ENV["CI_VERSION"] | |
| expected_ios_build = ENV["CI_IOS_BUILD"].to_i | |
| expected_android_build = ENV["CI_ANDROID_BUILD"].to_i | |
| version_matches = pkg_version == expected_version | |
| ios_matches = ios_build == expected_ios_build | |
| android_matches = android_build == expected_android_build | |
| ios_build = Integer(get_ios_build_number) | |
| android_build = Integer(get_android_build_number) | |
| expected_version = ENV["CI_VERSION"] | |
| expected_ios_build = Integer(ENV["CI_IOS_BUILD"]) | |
| expected_android_build = Integer(ENV["CI_ANDROID_BUILD"]) | |
| version_matches = pkg_version == expected_version | |
| ios_matches = ios_build == expected_ios_build | |
| android_matches = android_build == expected_android_build |
🤖 Prompt for AI Agents
In app/fastlane/helpers/version_manager.rb around lines 54 to 64, the
comparisons between build numbers can produce false mismatches because values
from JSON or ENV may be strings or nil; coerce and validate both sides using
Integer() for strict typing and fail-fast behavior: call
Integer(get_ios_build_number) and Integer(ENV["CI_IOS_BUILD"]) (and the same for
Android) before comparing, and rescue ArgumentError/TypeError to raise a clear
descriptive error if parsing fails so invalid inputs are surfaced immediately.
allow staging pull requests into main
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/mobile-deploy.yml (1)
1390-1463: Release tags point at the wrong commitTagging off
inputs.bump_target_branch(defaultdev) means the release tags land on whatever dev currently points to, not the commit we actually built (github.shaon the triggering branch, e.g. staging). That breaks traceability for production deploys: the published tags/releases no longer match the artifacts Fastlane uploaded. Check out the build commit (github.shaorgithub.ref_name) before tagging, so tags stay anchored to the code that shipped.
♻️ Duplicate comments (1)
.github/workflows/mobile-deploy.yml (1)
787-794: Fix iOS gems path in cache reportWe still report
${APP_PATH}/vendor/bundle, but iOS gems live under${APP_PATH}/ios/vendor/bundle(per the repo’s workflow convention). The current log underreports cache usage, making it harder to spot bundle blow‑ups before we hit GitHub’s quota. Point the size check atios/vendor/bundle.
Based on learnings
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/block-non-dev-to-main.yml(2 hunks).github/workflows/mobile-deploy.yml(18 hunks).github/workflows/release-calendar.yml(1 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/block-non-dev-to-main.yml.github/workflows/mobile-deploy.yml.github/workflows/release-calendar.yml
🧠 Learnings (2)
📚 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-deploy.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-bundler composite action for Ruby gems caching in workflows
Applied to files:
.github/workflows/mobile-deploy.yml
🪛 actionlint (1.7.7)
.github/workflows/block-non-dev-to-main.yml
12-12: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions for more details
(expression)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-deps
- GitHub Check: e2e-ios
- GitHub Check: analyze-android
- GitHub Check: analyze-ios
| - name: Block PRs not from staging | ||
| run: | | ||
| if [[ "${{ github.head_ref }}" != "dev" ]]; then | ||
| echo "You can only merge from dev to main." | ||
| if [[ "${{ github.head_ref }}" != "staging" ]]; then | ||
| echo "You can only merge from staging to main." |
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.
Harden the head_ref check against injection. Directly splicing ${{ github.head_ref }} into the bash script lets a malicious branch name trigger command substitution before the guard runs. Capture the value in an env var and reference that instead.
Apply this diff to fix the vulnerability:
steps:
- name: Block PRs not from staging
+ env:
+ HEAD_REF: ${{ github.head_ref }}
run: |
- if [[ "${{ github.head_ref }}" != "staging" ]]; then
+ if [[ "$HEAD_REF" != "staging" ]]; then
echo "You can only merge from staging to main."
exit 1
fi📝 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.
| - name: Block PRs not from staging | |
| run: | | |
| if [[ "${{ github.head_ref }}" != "dev" ]]; then | |
| echo "You can only merge from dev to main." | |
| if [[ "${{ github.head_ref }}" != "staging" ]]; then | |
| echo "You can only merge from staging to main." | |
| - name: Block PRs not from staging | |
| env: | |
| HEAD_REF: ${{ github.head_ref }} | |
| run: | | |
| if [[ "$HEAD_REF" != "staging" ]]; then | |
| echo "You can only merge from staging to main." | |
| exit 1 | |
| fi |
🧰 Tools
🪛 actionlint (1.7.7)
12-12: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions for more details
(expression)
🤖 Prompt for AI Agents
In .github/workflows/block-non-dev-to-main.yml around lines 11–14, the workflow
embeds `${{ github.head_ref }}` directly into the shell which can lead to
command substitution/injection; capture the value into a safe env var and use
that variable in the script instead. Add an env key (e.g. HEAD_REF: ${{
github.head_ref }}) for the step or export a local var from that env, then
reference it in the run block using a quoted variable (e.g. [[ "$HEAD_REF" !=
"staging" ]]) and echo via a safe expansion (printf '%s\n' "$HEAD_REF" or echo
"$HEAD_REF") so no branch name gets interpreted by the shell. Ensure the
comparison and echo are quoted to prevent word splitting and globbing.
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
| issues: write # Required for creating labels | ||
|
|
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.
Grant write access for git pushes.
release_to_staging pushes release/* branches back to origin, but the workflow declares contents: read. With only read scope the default GITHUB_TOKEN cannot push, so this job will fail every time it reaches the git push step. Bump the workflow (or that job) to contents: write so branch creation succeeds.
permissions:
- contents: read
+ contents: write
pull-requests: write
issues: write📝 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.
| permissions: | |
| contents: read | |
| pull-requests: write | |
| issues: write # Required for creating labels | |
| permissions: | |
| contents: write | |
| pull-requests: write | |
| issues: write # Required for creating labels |
🤖 Prompt for AI Agents
In .github/workflows/release-calendar.yml around lines 45 to 49 the workflow
sets permissions.contents to read but the release_to_staging job performs git
pushes; change the permissions scope to grant write access so the GITHUB_TOKEN
can push. Update the workflow (either globally or for that job) to set contents:
write and commit the workflow file so branch creation and git push steps
succeed.
| for LABEL in release automated staging; do | ||
| if ! gh label list --json name --jq '.[].name' | grep -q "^${LABEL}$"; then | ||
| echo "Creating missing label: ${LABEL}" | ||
| gh label create "${LABEL}" --color BFD4F2 | ||
| else | ||
| echo "Label ${LABEL} already exists." | ||
| fi | ||
| done |
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.
Make label lookup exhaustive before creating.
gh label list defaults to paginating the first page (30 entries). If the repo already has >30 labels and the release label happens to be outside page one, the script will miss it, then gh label create exits 1 with “already exists,” breaking the workflow. Add an explicit --limit (or use gh api) so you inspect all labels before attempting creation.
🤖 Prompt for AI Agents
.github/workflows/release-calendar.yml around lines 138 to 145: the label
existence check uses `gh label list` which paginates and may miss labels beyond
the default page, causing `gh label create` to fail when the label actually
exists; update the check to fetch all labels (for example add an explicit
`--limit` with a large number like `--limit 1000` to `gh label list`, or replace
the listing with a `gh api` call that retrieves all pages) so the lookup is
exhaustive before trying to create the label; keep the existing create path but
rely on the exhaustive lookup to avoid false creates (optionally handle `gh
label create` non-fatal exit if it still reports already exists).
Summary by CodeRabbit
Bug Fixes
Improvements
Chores