staging: test mobile deploy auto pull request (#1234) #1237
Conversation
* refine mobile deploy auto pr logic * remove env check * fix get-version
transphorm
changed the title
WalkthroughAdds a composite GitHub Action to create version-bump PRs, centralizes version bump detection and PR creation in the mobile-deploy workflow, introduces a Node-based version-manager script and updates Fastlane to read/verify CI-provided version/build numbers instead of performing local bumps. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor CI as GitHub Actions
participant BUMP as bump-version job
participant VM as version-manager.cjs
participant PR_ACTION as create-version-bump-pr (composite action)
participant GH as GitHub / gh CLI
CI->>BUMP: determine bump type (labels or input)
BUMP->>VM: getVersionInfo() / bump/apply if requested
BUMP-->>CI: outputs (version, ios_build, android_build, platform, version_bump_type)
CI->>PR_ACTION: inputs(platform, version, file_paths, github_token)
PR_ACTION->>PR_ACTION: git config, fetch staging/dev, compare commits
alt no new commits
PR_ACTION-->>CI: exit early (no PR)
else commits ahead
PR_ACTION->>GH: create branch from staging, push, gh pr create (base=bump_target_branch)
GH-->>CI: PR URL / result
end
sequenceDiagram
autonumber
participant Fastlane as Fastlane lanes
participant Helper as version_manager.rb
participant VM as version-manager.cjs
Fastlane->>Helper: verify_ci_version_match() (reads CI env)
alt CI values valid
Helper-->>Fastlane: {version, ios_build, android_build}
Fastlane->>Fastlane: use values for increment_build_number / increment_version_code (no local semantic bump)
else mismatch
Helper-->>Fastlane: raise CI version mismatch error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
app/fastlane/Fastfile (2)
104-106: Add error handling for the version bump script.The Node.js one-liner lacks error handling. If the
semverpackage is missing from dependencies or if the script fails for any reason, the failure won't be gracefully handled.Consider adding validation or error handling:
when "major", "minor", "patch" # Use Node.js with semver to bump version - sh("cd .. && node -e \"const fs = require('fs'); const pkg = require('./package.json'); const semver = require('semver'); pkg.version = semver.inc(pkg.version, '#{version_bump}'); fs.writeFileSync('./package.json', JSON.stringify(pkg, null, 2) + '\\n');\"") + begin + sh("cd .. && node -e \"const fs = require('fs'); const pkg = require('./package.json'); const semver = require('semver'); pkg.version = semver.inc(pkg.version, '#{version_bump}'); fs.writeFileSync('./package.json', JSON.stringify(pkg, null, 2) + '\\n');\"") + rescue => e + UI.user_error!("Failed to bump version: #{e.message}. Ensure 'semver' is in package.json dependencies.") + end UI.success("✅ Bumped #{version_bump} version")
307-309: Add error handling for the version bump script.The Node.js one-liner lacks error handling. If the
semverpackage is missing from dependencies or if the script fails for any reason, the failure won't be gracefully handled.Consider adding validation or error handling:
when "major", "minor", "patch" # Use Node.js with semver to bump version - sh("cd .. && node -e \"const fs = require('fs'); const pkg = require('./package.json'); const semver = require('semver'); pkg.version = semver.inc(pkg.version, '#{version_bump}'); fs.writeFileSync('./package.json', JSON.stringify(pkg, null, 2) + '\\n');\"") + begin + sh("cd .. && node -e \"const fs = require('fs'); const pkg = require('./package.json'); const semver = require('semver'); pkg.version = semver.inc(pkg.version, '#{version_bump}'); fs.writeFileSync('./package.json', JSON.stringify(pkg, null, 2) + '\\n');\"") + rescue => e + UI.user_error!("Failed to bump version: #{e.message}. Ensure 'semver' is in package.json dependencies.") + end UI.success("✅ Bumped #{version_bump} version").github/actions/create-version-bump-pr/action.yml (1)
36-50: Add error handling for git operations.Multiple git operations (checkout, fetch, cherry-pick, push) can fail but lack error handling. For example, the branch might already exist, cherry-pick could encounter conflicts, or the push could fail if the branch exists remotely.
Consider adding error handling and cleanup:
# Commit changes on temporary branch git checkout -b temp-version-commit git commit -m "chore: bump ${{ inputs.platform }} version for ${{ inputs.version }} [skip ci]" # Create new branch from dev git fetch origin dev - git checkout -b ${BRANCH_NAME} origin/dev + if git show-ref --verify --quiet refs/heads/${BRANCH_NAME}; then + echo "Branch ${BRANCH_NAME} already exists locally, deleting..." + git branch -D ${BRANCH_NAME} + fi + git checkout -b ${BRANCH_NAME} origin/dev # Cherry-pick only the version changes - git cherry-pick temp-version-commit + if ! git cherry-pick temp-version-commit; then + echo "❌ Cherry-pick failed. Aborting..." + git cherry-pick --abort + git checkout - + git branch -D temp-version-commit + exit 1 + fi # Clean up temporary branch git branch -D temp-version-commit # Push and create PR - git push --set-upstream origin ${BRANCH_NAME} + git push --force --set-upstream origin ${BRANCH_NAME}.github/workflows/mobile-deploy.yml (1)
155-173: Consider using jq for more robust label parsing.Using
grep -qon JSON output can be fragile if label names contain special characters or if the JSON structure changes. Consider usingjqfor more robust parsing:# Override with PR label if present if [ "${{ github.event_name }}" = "pull_request" ]; then LABELS='${{ toJSON(github.event.pull_request.labels.*.name) }}' - if echo "$LABELS" | grep -q "version:major"; then + if echo "$LABELS" | jq -e '.[] | select(. == "version:major")' > /dev/null; then VERSION_BUMP="major" - elif echo "$LABELS" | grep -q "version:minor"; then + elif echo "$LABELS" | jq -e '.[] | select(. == "version:minor")' > /dev/null; then VERSION_BUMP="minor" - elif echo "$LABELS" | grep -q "version:patch"; then + elif echo "$LABELS" | jq -e '.[] | select(. == "version:patch")' > /dev/null; then VERSION_BUMP="patch" fi fiThis is more precise and handles edge cases better, though the current implementation should work for simple label names.
Also applies to: 746-764
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/actions/create-version-bump-pr/action.yml(1 hunks).github/workflows/mobile-deploy.yml(5 hunks)app/fastlane/Fastfile(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.github/workflows/**/*.{yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
.github/workflows/**/*.{yml,yaml}: In GitHub workflows, use the shared composite actions in .github/actions for dependency caching instead of calling actions/cache directly
Use the cache-yarn composite action for Yarn dependency caching in workflows
Use the cache-bundler composite action for Ruby gems caching in workflows
Use the cache-gradle composite action for Gradle caching in workflows
Use the cache-pods composite action for CocoaPods caching in workflows
Files:
.github/workflows/mobile-deploy.yml
🪛 actionlint (1.7.7)
.github/workflows/mobile-deploy.yml
1058-1058: property "version" is not defined in object type {}
(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). (5)
- GitHub Check: analyze-ios
- GitHub Check: build-deps
- GitHub Check: analyze-android
- GitHub Check: e2e-ios
- GitHub Check: android-build-test
🔇 Additional comments (1)
.github/workflows/mobile-deploy.yml (1)
1069-1074: Clarify release tagging dependency on PR creation.The
create-release-tagsjob depends oncreate-version-bump-pr, but the version bump PR needs to be merged before creating release tags. This dependency ensures the PR is created successfully but doesn't ensure it's merged.Is the intent to:
- Create tags immediately after the PR is created (current behavior), or
- Create tags only after the version bump PR is merged?
If (2), this job should be triggered by a separate workflow that runs when the version bump PR is merged to dev. The current dependency chain doesn't ensure the PR is merged before creating tags.
…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
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
.github/workflows/mobile-e2e.yml (3)
140-144: Avoid listing private repo contents in CI logs.
ls -la app/android/android-passport-nfc-reader | head -10can leak private repo structure in public logs. Prefer a presence check or a specific file assertion without directory listing.Apply this change:
- echo "📁 android-passport-nfc-reader contents:" - ls -la app/android/android-passport-nfc-reader/ | head -10 + # Verify expected files without listing the directory + test -f app/android/android-passport-nfc-reader/README.md || echo "ℹ️ README not present (ok)"
118-123: Add Gradle wrapper validation to mitigate supply‑chain risks.Validate the Gradle wrapper before executing to prevent tampering.
Insert this step before building:
- name: Build Android APK + - name: Validate Gradle Wrapper + uses: gradle/wrapper-validation-action@v2 + with: + min-wrapper-count: 1 + allow-snapshots: false + - name: Build Android APK run: | echo "Building Android APK..." chmod +x app/android/gradlew (cd app/android && ./gradlew assembleDebug --quiet --parallel --build-cache --no-configuration-cache) || { echo "❌ Android build failed"; exit 1; } echo "✅ Android build succeeded"
241-243: Pin Maestro installer to a fixed version
Unpinnedcurl | bashis a supply-chain risk. Define a stableMAESTRO_VERSION(e.g. in job-levelenv) and use it in the install step per official docs; optionally verify checksum..github/workflows/mobile-e2e.yml @@ -240,7 +240,11 @@ jobs: # … - - 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' + env: + MAESTRO_VERSION: ${{ env.MAESTRO_VERSION }} + run: | + curl -Ls "https://get.maestro.mobile.dev" | bash + maestro --version + at top of workflow: + env: + MAESTRO_VERSION: 1.2.3 # pin to a specific release.github/workflows/mobile-deploy.yml (1)
381-381: Scope disabling Yarn hardened mode to only the failing command
Global export ofYARN_ENABLE_HARDENED_MODE=0weakens supply‐chain protections. Identify which step fails under hardened mode and apply the override only to that command (e.g.YARN_ENABLE_HARDENED_MODE=0 yarn install) or fix the underlying issue to preserve hardened mode.
🧹 Nitpick comments (2)
app/fastlane/helpers/version_manager.rb (1)
30-35: Add error handling for package.json readsget_current_version raises low-signal exceptions if package.json is missing/invalid. Mirror read_version_file’s guard/handling for reliability.
Outside the shown lines, consider:
def get_current_version package_json_path = File.expand_path("../../package.json", __dir__) unless File.exist?(package_json_path) UI.user_error!("package.json not found at #{package_json_path}") end JSON.parse(File.read(package_json_path))["version"] rescue JSON::ParserError => e UI.user_error!("Failed to parse package.json: #{e.message}") endapp/scripts/version-manager.cjs (1)
89-100: Validate version.json shape and numeric types before useGuard against missing keys or non-numeric builds to fail fast with actionable errors. Prevents downstream CI/Fastlane mismatches.
Apply this diff:
function getVersionInfo() { const pkg = readPackageJson(); const versionData = readVersionJson(); + if (!versionData || typeof versionData !== 'object') { + throw new Error('version.json: invalid structure'); + } + const ios = versionData.ios; + const android = versionData.android; + if (!ios || typeof ios.build !== 'number') { + throw new Error('version.json: ios.build must be a number'); + } + if (!android || typeof android.build !== 'number') { + throw new Error('version.json: android.build must be a number'); + } + return { version: pkg.version, iosBuild: versionData.ios.build, androidBuild: versionData.android.build, iosLastDeployed: versionData.ios.lastDeployed, androidLastDeployed: versionData.android.lastDeployed, }; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.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)app/fastlane/Fastfile(6 hunks)app/fastlane/helpers/version_manager.rb(1 hunks)app/scripts/version-manager.cjs(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/mobile-e2e.yml.github/workflows/mobile-deploy.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
🪛 RuboCop (1.81.1)
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)
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)
⏰ 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). (5)
- GitHub Check: build-deps
- GitHub Check: android-build-test
- GitHub Check: e2e-ios
- GitHub Check: analyze-android
- GitHub Check: analyze-ios
🔇 Additional comments (2)
.github/workflows/mobile-e2e.yml (2)
37-37: Timeout increase to 120m: OK — monitor runner utilization.Looks reasonable for heavier builds. Keep an eye on queue times and job cancellations under the concurrency group.
181-181: iOS timeout to 120m: OK — confirm org limits on macOS large runners.macos-latest-large capacity/time quotas vary by plan; ensure this won’t throttle other pipelines.
| # Ensure we're on staging branch, not detached HEAD | ||
| git fetch origin staging dev | ||
| git checkout staging | ||
| # Check if staging has commits not in dev (version bumps + any build changes) | ||
| COMMITS_AHEAD=$(git rev-list --count origin/dev..staging) | ||
| if [ "$COMMITS_AHEAD" -eq 0 ]; then | ||
| echo "ℹ️ No new commits on staging compared to dev. Skipping PR creation." |
There was a problem hiding this comment.
Make branch checkout and ahead-count robust against detached HEADs
git checkout staging can fail if no local branch exists. Also compute ahead using remote refs to avoid local divergence.
Apply this diff:
- # Ensure we're on staging branch, not detached HEAD
- git fetch origin staging dev
- git checkout staging
+ # Ensure we're on staging branch (freshly tracking remote), not detached HEAD
+ set -euo pipefail
+ git fetch --prune origin
+ git checkout -B staging origin/staging
@@
- COMMITS_AHEAD=$(git rev-list --count origin/dev..staging)
+ COMMITS_AHEAD=$(git rev-list --count origin/dev..origin/staging)Also applies to: 40-47
🤖 Prompt for AI Agents
.github/actions/create-version-bump-pr/action.yml around lines 28-36 (and
similarly apply to lines 40-47): make the branch checkout robust by creating or
resetting the local staging branch from the remote instead of a plain checkout,
and compute the "commits ahead" count using remote refs only (compare origin/dev
to origin/staging) to avoid relying on local branch state or a detached HEAD;
update the script to fetch remotes, force-create/reset the local staging branch
to origin/staging, and replace the rev-list range with the
origin/dev..origin/staging remote-only range, and apply the same changes to the
other occurrence at lines 40-47.
| id: get-version | ||
| shell: bash | ||
| run: | | ||
| VERSION=$(node -p "require('${{ inputs.app_path }}/package.json').version") | ||
| echo "version=$VERSION" >> $GITHUB_OUTPUT | ||
| echo "VERSION=$VERSION" >> $GITHUB_ENV |
There was a problem hiding this comment.
Fix Node require path to avoid resolving a module instead of a file
require('${{ inputs.app_path }}/package.json') may resolve a package in node_modules if app_path doesn’t start with ./ or /. Use path.resolve and strict shell flags.
Apply this diff:
- - name: Get version from package.json
- id: get-version
- shell: bash
- run: |
- VERSION=$(node -p "require('${{ inputs.app_path }}/package.json').version")
- echo "version=$VERSION" >> $GITHUB_OUTPUT
- echo "VERSION=$VERSION" >> $GITHUB_ENV
+ - name: Get version from package.json
+ id: get-version
+ shell: bash
+ run: |
+ set -euo pipefail
+ VERSION=$(node -p "require(require('path').resolve('${{ inputs.app_path }}', 'package.json')).version")
+ echo "version=$VERSION" >> \"$GITHUB_OUTPUT\"
+ echo "VERSION=$VERSION" >> \"$GITHUB_ENV\"Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
.github/actions/get-version/action.yml lines 19-24: the current shell command
uses node -p "require('${{ inputs.app_path }}/package.json').version" which can
cause Node to resolve a package from node_modules when app_path is not a file
path and lacks strict shell error handling; change the script to enable strict
shell flags (set -euo pipefail) and use Node to require a resolved absolute path
(e.g., path.resolve(process.cwd(), inputPath, 'package.json')) or otherwise call
require with a fully resolved path so Node loads the intended file, and
propagate any errors so the action fails fast.
| outputs: | ||
| version: ${{ steps.bump.outputs.version }} | ||
| ios_build: ${{ steps.bump.outputs.ios_build }} | ||
| android_build: ${{ steps.bump.outputs.android_build }} | ||
| version_bump_type: ${{ steps.determine-bump.outputs.version_bump }} | ||
| platform: ${{ steps.determine-platform.outputs.platform }} |
There was a problem hiding this comment.
🧩 Analysis chain
bump-version step outputs are not emitted (breaks downstream).
Job outputs map to steps.bump.outputs.*, but the bump run step doesn’t write to $GITHUB_OUTPUT. Downstream uses of needs.bump-version.outputs.version/builds will be empty, causing apply/build steps and PR/tagging to misbehave.
Fix by emitting outputs after the bump calculation (parse app/version.json as source of truth):
- name: Bump version using version-manager script
id: bump
run: |
cd ${{ env.APP_PATH }}
VERSION_BUMP="${{ steps.determine-bump.outputs.version_bump }}"
PLATFORM="${{ steps.determine-platform.outputs.platform }}"
echo "🔄 Calculating version bump..."
echo " Type: $VERSION_BUMP"
echo " Platform: $PLATFORM"
echo ""
# Use version-manager script to calculate bump
# NOTE: Using absolute path to ensure script is found regardless of CWD
node ${{ env.APP_PATH }}/scripts/version-manager.cjs bump "$VERSION_BUMP" "$PLATFORM"
echo ""
echo "✅ Version bump calculated successfully"
echo "⚠️ Note: Changes are local only. Will be committed in PR after successful builds."
+
+ # Export bump results for downstream jobs
+ VERSION=$(node -e "console.log(require('./version.json').version)")
+ IOS_BUILD=$(node -e "console.log(require('./version.json').ios?.build ?? '')")
+ ANDROID_BUILD=$(node -e "console.log(require('./version.json').android?.build ?? '')")
+
+ if [ -z "$VERSION" ] || [ -z "$IOS_BUILD" ] || [ -z "$ANDROID_BUILD" ]; then
+ echo "❌ Failed to read version/builds from version.json"
+ exit 1
+ fi
+ {
+ echo "version=$VERSION"
+ echo "ios_build=$IOS_BUILD"
+ echo "android_build=$ANDROID_BUILD"
+ } >> "$GITHUB_OUTPUT"To confirm the Node script already sets outputs (if that’s the intended design), run:
Also applies to: 223-243
🏁 Script executed:
#!/bin/bash
# Verify version-manager.cjs writes to $GITHUB_OUTPUT
fd -a version-manager.cjs
rg -n "GITHUB_OUTPUT|set-output" -S --type js --type tsLength of output: 50
Emit bump-version step outputs for downstream jobs
Downstream mappings needs.bump-version.outputs.* are empty because the bump step never writes to $GITHUB_OUTPUT, breaking version tags, builds and PR automation. After running node …/version-manager.cjs bump …, add:
+ # Export bump results for downstream jobs
+ VERSION=$(node -e "console.log(require('./version.json').version)")
+ IOS_BUILD=$(node -e "console.log(require('./version.json').ios?.build ?? '')")
+ ANDROID_BUILD=$(node -e "console.log(require('./version.json').android?.build ?? '')")
+ if [ -z "$VERSION" ] || [ -z "$IOS_BUILD" ] || [ -z "$ANDROID_BUILD" ]; then
+ echo "❌ Failed to read version/builds from version.json" && exit 1
+ fi
+ {
+ echo "version=$VERSION"
+ echo "ios_build=$IOS_BUILD"
+ echo "android_build=$ANDROID_BUILD"
+ } >> "$GITHUB_OUTPUT"Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
.github/workflows/mobile-deploy.yml around lines 156-161: the bump step invoked
via node does not write any values to $GITHUB_OUTPUT, so downstream mappings
(needs.bump-version.outputs.*) are empty; after running the node
version-manager.cjs bump command capture its JSON output (or parse stdout) and
append the required keys to $GITHUB_OUTPUT (e.g., version, ios_build,
android_build, version_bump, platform) using the recommended echo "key=value" >>
$GITHUB_OUTPUT pattern so the step with id "bump" exposes those outputs for
downstream jobs.
| if [ -d "${{ env.APP_PATH }}/vendor/bundle" ]; then | ||
| GEMS_SIZE=$(du -sh "${{ env.APP_PATH }}/vendor/bundle" | cut -f1) | ||
| echo "Ruby gems: $GEMS_SIZE" | ||
| fi |
There was a problem hiding this comment.
Fix Ruby gems path in cache usage reports (iOS vs Android).
The iOS job reports from app/vendor/bundle and Android from app/ios/vendor/bundle, which is inverted. Use platform-correct paths to avoid misleading diagnostics.
Based on learnings
- 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- if [ -d "${{ env.APP_PATH }}/ios/vendor/bundle" ]; then
- GEMS_SIZE=$(du -sh "${{ env.APP_PATH }}/ios/vendor/bundle" | cut -f1)
+ if [ -d "${{ env.APP_PATH }}/vendor/bundle" ]; then
+ GEMS_SIZE=$(du -sh "${{ env.APP_PATH }}/vendor/bundle" | cut -f1)
echo "Ruby gems: $GEMS_SIZE"
fiAlso applies to: 1207-1210
🤖 Prompt for AI Agents
.github/workflows/mobile-deploy.yml around lines 791-794: the script currently
checks "${{ env.APP_PATH }}/vendor/bundle" for the iOS job but that path is
inverted (iOS should check "${{ env.APP_PATH }}/ios/vendor/bundle" and Android
should check "${{ env.APP_PATH }}/vendor/bundle"); update the path used in this
iOS block to "${{ env.APP_PATH }}/ios/vendor/bundle" and make the corresponding
inversion fix in the similar block at lines 1207-1210 so each platform reports
the correct vendor/bundle location.
| # Push all tags (force to handle any conflicts) | ||
| if git push origin --tags 2>/dev/null; then | ||
| echo "🚀 Tags pushed to repository" | ||
| else | ||
| echo "⚠️ Some tags may already exist on remote, trying force push..." | ||
| git push origin --tags --force | ||
| echo "🚀 Tags force-pushed to repository" | ||
| fi |
There was a problem hiding this comment.
Avoid force-pushing all tags; push only the tags created by this run.
git push origin --tags with a force fallback can overwrite existing tags and push unrelated tags. This is risky for release integrity.
Push only the intended tags and never force:
- # Push all tags (force to handle any conflicts)
- if git push origin --tags 2>/dev/null; then
- echo "🚀 Tags pushed to repository"
- else
- echo "⚠️ Some tags may already exist on remote, trying force push..."
- git push origin --tags --force
- echo "🚀 Tags force-pushed to repository"
- fi
+ # Push only tags created in this run; skip if they already exist remotely
+ push_tag_safe() {
+ local TAG="$1"
+ if git ls-remote --tags origin "refs/tags/${TAG}" | grep -q "${TAG}"; then
+ echo "⏭️ ${TAG} already exists on remote"
+ else
+ git push origin "${TAG}"
+ echo "🚀 Pushed ${TAG}"
+ fi
+ }
+
+ push_tag_safe "v${VERSION}"
+ if [ "${{ needs.build-ios.result }}" = "success" ]; then
+ push_tag_safe "v${VERSION}-ios-${IOS_BUILD}"
+ fi
+ if [ "${{ needs.build-android.result }}" = "success" ]; then
+ push_tag_safe "v${VERSION}-android-${ANDROID_BUILD}"
+ 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.
| # Push all tags (force to handle any conflicts) | |
| if git push origin --tags 2>/dev/null; then | |
| echo "🚀 Tags pushed to repository" | |
| else | |
| echo "⚠️ Some tags may already exist on remote, trying force push..." | |
| git push origin --tags --force | |
| echo "🚀 Tags force-pushed to repository" | |
| fi | |
| # Push only tags created in this run; skip if they already exist remotely | |
| push_tag_safe() { | |
| local TAG="$1" | |
| if git ls-remote --tags origin "refs/tags/${TAG}" | grep -q "${TAG}"; then | |
| echo "⏭️ ${TAG} already exists on remote" | |
| else | |
| git push origin "${TAG}" | |
| echo "🚀 Pushed ${TAG}" | |
| fi | |
| } | |
| push_tag_safe "v${VERSION}" | |
| if [ "${{ needs.build-ios.result }}" = "success" ]; then | |
| push_tag_safe "v${VERSION}-ios-${IOS_BUILD}" | |
| fi | |
| if [ "${{ needs.build-android.result }}" = "success" ]; then | |
| push_tag_safe "v${VERSION}-android-${ANDROID_BUILD}" | |
| fi |
🤖 Prompt for AI Agents
.github/workflows/mobile-deploy.yml around lines 1453 to 1460: current logic
force-pushes all tags which can overwrite unrelated tags; instead capture the
specific tags created in this workflow run (e.g., collect tag names produced
earlier or compare repo state before/after to list newly created tags), then if
the list is non-empty push those tags explicitly with `git push origin <tag1>
<tag2> ...` and do not use --force; if no new tags were created, skip pushing
and log that no tags need to be pushed.
| # In CI, version should already be set by version-manager.cjs | ||
| # Verify it matches expected values | ||
| if is_ci && version_bump == "skip" | ||
| Fastlane::Helpers.verify_ci_version_match | ||
| end | ||
|
|
||
| # Prepare and build | ||
| result = prepare_ios_build(prod_release: deployment_track == "production") | ||
| # Prepare and build (no version bumping inside) | ||
| result = prepare_ios_build(prod_release: deployment_track == "production", version_bump: version_bump) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Verify helper namespace usage (Helpers vs Helpers::VersionManager)
Calls like Fastlane::Helpers.verify_ci_version_match and get_{ios,android}_build_number assume these are exposed on Helpers. In this PR, they are defined under Helpers::VersionManager. If wrappers aren’t present, lanes will crash at runtime.
Run this to confirm wrapper methods exist on Fastlane::Helpers:
If wrappers are missing, either update call sites to Fastlane::Helpers::VersionManager.* or add delegating methods on Helpers.
Also applies to: 175-178, 291-295, 332-335
🏁 Script executed:
#!/bin/bash
# Verify wrappers on Fastlane::Helpers
rg -nPU '(?s)^\s*module\s+Fastlane\b.*?^\s*module\s+Helpers\b.*?^\s*def\s+(verify_ci_version_match|get_ios_build_number|get_android_build_number|update_deployment_timestamp)\b' -g 'app/fastlane/**'Length of output: 2447
Fix helper namespace usage
Calls to Fastlane::Helpers.verify_ci_version_match, get_ios_build_number, get_android_build_number, and update_deployment_timestamp reference methods defined under Fastlane::Helpers::VersionManager. Either add delegating methods in Fastlane::Helpers or update these call sites to Fastlane::Helpers::VersionManager.* (app/fastlane/Fastfile lines 98-106, 175-178, 291-295, 332-335)
🤖 Prompt for AI Agents
In app/fastlane/Fastfile around lines 98-106 (and also update call sites at
~175-178, ~291-295, ~332-335), calls currently refer to methods on
Fastlane::Helpers (e.g., Fastlane::Helpers.verify_ci_version_match,
get_ios_build_number, get_android_build_number, update_deployment_timestamp) but
the implementations live under Fastlane::Helpers::VersionManager; change those
call sites to call Fastlane::Helpers::VersionManager.method_name (or
alternatively add thin delegating methods on Fastlane::Helpers that forward to
Fastlane::Helpers::VersionManager) so the correct namespace is used
consistently.
| export_method: "app-store", | ||
| output_directory: "build", | ||
| output_directory: "ios/build", | ||
| clean: true, |
There was a problem hiding this comment.
Fix iOS output_directory path (wrong CWD)
"ios/build" is relative and likely resolves under app/fastlane, not app/ios. Use a path anchored to this file to avoid build output in the wrong place.
Apply this diff:
- output_directory: "ios/build",
+ output_directory: File.expand_path("../ios/build", __dir__),Recommend normalizing other iOS/Android paths similarly (xcodeproj, Podfile, gradle_file_path, project_dir) to use File.expand_path with dir for deterministic paths.
🤖 Prompt for AI Agents
In app/fastlane/Fastfile around line 228 the output_directory is set to a
relative "ios/build" which will resolve against the Fastfile CWD (app/fastlane)
and write build artifacts to the wrong folder; change it to an absolute path
anchored to this Fastfile using File.expand_path(File.join(__dir__, '..', 'ios',
'build')) (or equivalent) so the path always points to app/ios/build, and apply
the same pattern (File.expand_path with __dir__) to other iOS/Android paths
(xcodeproj, Podfile, gradle_file_path, project_dir) to ensure deterministic,
correct locations.
| pkg_version = get_current_version | ||
| 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 | ||
|
|
There was a problem hiding this comment.
Harden type handling to avoid false mismatches and ambiguous failures
Coerce build numbers to integers on both sides and use ENV.fetch for clearer errors. This prevents subtle mismatches if version.json contains strings and surfaces missing ENV early.
Apply this diff:
- pkg_version = get_current_version
- ios_build = get_ios_build_number
- android_build = get_android_build_number
+ pkg_version = get_current_version
+ ios_build = Integer(get_ios_build_number)
+ android_build = Integer(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
+ expected_version = ENV.fetch("CI_VERSION")
+ expected_ios_build = Integer(ENV.fetch("CI_IOS_BUILD"))
+ expected_android_build = Integer(ENV.fetch("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.
| pkg_version = get_current_version | |
| 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 | |
| pkg_version = get_current_version | |
| ios_build = Integer(get_ios_build_number) | |
| android_build = Integer(get_android_build_number) | |
| expected_version = ENV.fetch("CI_VERSION") | |
| expected_ios_build = Integer(ENV.fetch("CI_IOS_BUILD")) | |
| expected_android_build = Integer(ENV.fetch("CI_ANDROID_BUILD")) |
🤖 Prompt for AI Agents
In app/fastlane/helpers/version_manager.rb around lines 53 to 60, the code
currently reads ENV[...] and compares build numbers that may be strings; update
it to use ENV.fetch("CI_VERSION") and ENV.fetch("CI_IOS_BUILD").to_i /
ENV.fetch("CI_ANDROID_BUILD").to_i so missing CI vars raise clear errors, and
coerce the values returned by get_ios_build_number and get_android_build_number
to integers (e.g. .to_i) before comparison to avoid false mismatches when
version.json contains strings.
There was a problem hiding this comment.
Actionable comments posted: 8
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)
1207-1210: Fix Ruby gems cache path for Android monitoring.The monitoring step in the Android build job checks
app/ios/vendor/bundle, but Android builds cache Ruby gems atapp/vendor/bundle(as configured in line 958 and confirmed by learnings). This reports the wrong cache size.Based on learnings
Apply this diff:
- if [ -d "${{ env.APP_PATH }}/ios/vendor/bundle" ]; then - GEMS_SIZE=$(du -sh "${{ env.APP_PATH }}/ios/vendor/bundle" | cut -f1) + if [ -d "${{ env.APP_PATH }}/vendor/bundle" ]; then + GEMS_SIZE=$(du -sh "${{ env.APP_PATH }}/vendor/bundle" | cut -f1) echo "Ruby gems: $GEMS_SIZE" fi
🧹 Nitpick comments (2)
.github/workflows/mobile-deploy.yml (2)
236-238: Clarify path terminology in comment.The comment states "Using absolute path" but
${{ env.APP_PATH }}expands to${{ github.workspace }}/app, which is a fully-qualified path, not an absolute path in the traditional sense. While the path works correctly, the comment could be clearer.Update the comment:
- # Use version-manager script to calculate bump - # NOTE: Using absolute path to ensure script is found regardless of CWD + # Use version-manager script to calculate bump (with full path for reliability) node ${{ env.APP_PATH }}/scripts/version-manager.cjs bump "$VERSION_BUMP" "$PLATFORM"
1409-1460: Consider checking tag existence before attempting creation.While the current approach handles errors gracefully, relying on git exit codes (code 128 for existing tags) is brittle and could change across git versions. The force push fallback at the end could also be risky if multiple workflows attempt to push simultaneously.
Consider checking tag existence first:
echo "📦 Creating tags for version $VERSION" # Create main version tag (idempotent) - if git tag -a "v${VERSION}" -m "Release ${VERSION}" 2>/dev/null; then - echo "✅ Created tag: v${VERSION}" - else - EXIT_CODE=$? - if [ $EXIT_CODE -eq 128 ]; then - echo "⏭️ Tag v${VERSION} already exists" - else - echo "❌ Failed to create tag v${VERSION} with exit code $EXIT_CODE" - exit 1 - fi + if git rev-parse "v${VERSION}" >/dev/null 2>&1; then + echo "⏭️ Tag v${VERSION} already exists" + else + git tag -a "v${VERSION}" -m "Release ${VERSION}" + echo "✅ Created tag: v${VERSION}" fiApply similar pattern to the platform-specific tags and use
--force-with-leasefor safer concurrent pushes:- # Push all tags (force to handle any conflicts) - if git push origin --tags 2>/dev/null; then - echo "🚀 Tags pushed to repository" - else - echo "⚠️ Some tags may already exist on remote, trying force push..." - git push origin --tags --force - echo "🚀 Tags force-pushed to repository" - fi + # Push tags (using force-with-lease for safety) + git push origin --tags --force-with-lease + echo "🚀 Tags pushed to repository"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.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)app/fastlane/Fastfile(6 hunks)app/fastlane/helpers/version_manager.rb(1 hunks)app/scripts/version-manager.cjs(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/mobile-e2e.yml.github/workflows/mobile-deploy.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
🪛 RuboCop (1.81.1)
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)
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)
⏰ 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). (5)
- GitHub Check: build-deps
- GitHub Check: android-build-test
- GitHub Check: e2e-ios
- GitHub Check: analyze-android
- GitHub Check: analyze-ios
🔇 Additional comments (6)
.github/workflows/mobile-e2e.yml (1)
37-37: Timeout increases look fineLonger builds/e2e can justify 120m. No security or performance risk introduced.
Also applies to: 181-181
.github/actions/get-version/action.yml (1)
10-14: Good addition: expose version via outputsExporting to GITHUB_OUTPUT with a stable step id improves reuse; keeping GITHUB_ENV preserves backward compatibility.
Also applies to: 19-19, 23-23
app/fastlane/helpers/version_manager.rb (1)
47-78: CI version verification is solidStrict env checks + explicit comparisons + clear error logs. This reduces release mistakes. No medium+ issues spotted.
app/fastlane/Fastfile (2)
98-102: Verification gate in CI is appropriateConditionally verifying CI-provided version/build before build is a good safety check.
175-184: Syncing build numbers from version.json is correctSetting Xcode build number and Android versionCode from CI-managed data matches the new flow. iOS build-number verification present; Android verification is acknowledged and gated by permissions.
Also applies to: 332-340
.github/workflows/mobile-deploy.yml (1)
287-309: LGTM: Branch verification logic is solid.The branch verification steps correctly handle both PR merges (building from base branch) and manual triggers (building from ref_name), with clear logging and defensive checks to detect potential issues.
| - name: Create version bump PR | ||
| shell: bash | ||
| run: | | ||
| BRANCH_NAME="ci/bump-${{ inputs.platform }}-build-${{ github.run_id }}" | ||
| git config user.name "github-actions[bot]" | ||
| git config user.email "github-actions[bot]@users.noreply.github.com" | ||
| # Ensure we're on staging branch, not detached HEAD | ||
| git fetch origin staging dev | ||
| git checkout staging | ||
| # Check if staging has commits not in dev (version bumps + any build changes) | ||
| COMMITS_AHEAD=$(git rev-list --count origin/dev..staging) | ||
| if [ "$COMMITS_AHEAD" -eq 0 ]; then | ||
| echo "ℹ️ No new commits on staging compared to dev. Skipping PR creation." | ||
| exit 0 | ||
| fi | ||
| echo "📊 Staging is $COMMITS_AHEAD commit(s) ahead of dev" | ||
| # Create new branch from current staging (which has all version changes) | ||
| git checkout -b ${BRANCH_NAME} | ||
| # Push the branch | ||
| git push --set-upstream origin ${BRANCH_NAME} | ||
| # Determine PR title based on platform | ||
| if [ "${{ inputs.platform }}" = "mobile" ]; then | ||
| PR_TITLE="chore: bump mobile app version to ${{ inputs.version }}" | ||
| else | ||
| PR_TITLE="chore: bump ${{ inputs.platform }} build for ${{ inputs.version }}" | ||
| fi | ||
| gh pr create \ | ||
| --base dev \ | ||
| --head ${BRANCH_NAME} \ | ||
| --title "$PR_TITLE" \ | ||
| --body "Automated version bump by CI" \ | ||
| --label "automated" | ||
| env: |
There was a problem hiding this comment.
Harden the script with bash strict mode; unused input is misleading
- Without set -euo pipefail, errors in intermediary commands can slip by, creating or labeling an unintended PR.
- inputs.file_paths is declared but unused; this is confusing and may imply partial-change PRs that the action doesn’t implement.
Apply this diff to harden the script:
- name: Create version bump PR
shell: bash
run: |
+ set -euo pipefail
BRANCH_NAME="ci/bump-${{ inputs.platform }}-build-${{ github.run_id }}"And either remove the file_paths input from the action definition, or document it as unused. If you intend to restrict PRs to certain paths, implement staged commits based on the provided list before pushing.
📝 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: Create version bump PR | |
| shell: bash | |
| run: | | |
| BRANCH_NAME="ci/bump-${{ inputs.platform }}-build-${{ github.run_id }}" | |
| git config user.name "github-actions[bot]" | |
| git config user.email "github-actions[bot]@users.noreply.github.com" | |
| # Ensure we're on staging branch, not detached HEAD | |
| git fetch origin staging dev | |
| git checkout staging | |
| # Check if staging has commits not in dev (version bumps + any build changes) | |
| COMMITS_AHEAD=$(git rev-list --count origin/dev..staging) | |
| if [ "$COMMITS_AHEAD" -eq 0 ]; then | |
| echo "ℹ️ No new commits on staging compared to dev. Skipping PR creation." | |
| exit 0 | |
| fi | |
| echo "📊 Staging is $COMMITS_AHEAD commit(s) ahead of dev" | |
| # Create new branch from current staging (which has all version changes) | |
| git checkout -b ${BRANCH_NAME} | |
| # Push the branch | |
| git push --set-upstream origin ${BRANCH_NAME} | |
| # Determine PR title based on platform | |
| if [ "${{ inputs.platform }}" = "mobile" ]; then | |
| PR_TITLE="chore: bump mobile app version to ${{ inputs.version }}" | |
| else | |
| PR_TITLE="chore: bump ${{ inputs.platform }} build for ${{ inputs.version }}" | |
| fi | |
| gh pr create \ | |
| --base dev \ | |
| --head ${BRANCH_NAME} \ | |
| --title "$PR_TITLE" \ | |
| --body "Automated version bump by CI" \ | |
| --label "automated" | |
| env: | |
| - name: Create version bump PR | |
| shell: bash | |
| run: | | |
| set -euo pipefail | |
| BRANCH_NAME="ci/bump-${{ inputs.platform }}-build-${{ github.run_id }}" | |
| git config user.name "github-actions[bot]" | |
| git config user.email "github-actions[bot]@users.noreply.github.com" | |
| # Ensure we're on staging branch, not detached HEAD | |
| git fetch origin staging dev | |
| git checkout staging | |
| # Check if staging has commits not in dev (version bumps + any build changes) | |
| COMMITS_AHEAD=$(git rev-list --count origin/dev..staging) | |
| if [ "$COMMITS_AHEAD" -eq 0 ]; then | |
| echo "ℹ️ No new commits on staging compared to dev. Skipping PR creation." | |
| exit 0 | |
| fi | |
| echo "📊 Staging is $COMMITS_AHEAD commit(s) ahead of dev" | |
| # Create new branch from current staging (which has all version changes) | |
| git checkout -b ${BRANCH_NAME} | |
| # Push the branch | |
| git push --set-upstream origin ${BRANCH_NAME} | |
| # Determine PR title based on platform | |
| if [ "${{ inputs.platform }}" = "mobile" ]; then | |
| PR_TITLE="chore: bump mobile app version to ${{ inputs.version }}" | |
| else | |
| PR_TITLE="chore: bump ${{ inputs.platform }} build for ${{ inputs.version }}" | |
| fi | |
| gh pr create \ | |
| --base dev \ | |
| --head ${BRANCH_NAME} \ | |
| --title "$PR_TITLE" \ | |
| --body "Automated version bump by CI" \ | |
| --label "automated" | |
| env: |
🤖 Prompt for AI Agents
.github/actions/create-version-bump-pr/action.yml lines 20-61: the action's run
script lacks bash strict mode and declares an unused inputs.file_paths which is
misleading; enable strict mode (set -euo pipefail and IFS=$'\n\t') at the top of
the script and ensure variables are quoted to fail fast on errors, and then
either remove inputs.file_paths from the action.yml inputs or document it as
unused; if you intend to limit changes by path, implement staging/committing
only the listed paths before creating the branch and pushing.
| git fetch origin staging dev | ||
| git checkout staging | ||
| # Check if staging has commits not in dev (version bumps + any build changes) | ||
| COMMITS_AHEAD=$(git rev-list --count origin/dev..staging) | ||
There was a problem hiding this comment.
Ensure you base the branch and ahead-count on origin/staging (avoid stale local branch)
Using a possibly stale local staging branch risks false “no commits” and wrong diffs. Reset to the remote ref and compute ahead against origin/staging.
Apply this diff:
- git fetch origin staging dev
- git checkout staging
+ git fetch origin staging dev --prune
+ # Ensure local 'staging' matches the remote ref exactly
+ git checkout -B staging origin/staging
@@
- COMMITS_AHEAD=$(git rev-list --count origin/dev..staging)
+ COMMITS_AHEAD=$(git rev-list --count origin/dev..origin/staging)
@@
- git checkout -b ${BRANCH_NAME}
+ git checkout -b "${BRANCH_NAME}"Also applies to: 42-47
🤖 Prompt for AI Agents
In .github/actions/create-version-bump-pr/action.yml around lines 29 to 34 (and
similarly for lines 42 to 47), the script checks a possibly stale local staging
branch and computes commits-ahead against a local ref; update the workflow to
reset the local staging to the remote by fetching and checking out
origin/staging (or using git fetch origin staging && git checkout -B staging
origin/staging) and compute the ahead count against origin/staging (e.g., git
rev-list --count origin/dev..origin/staging) so the branch base and the
ahead-count are always computed from the remote refs rather than stale local
branches.
| 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.
Fix Ruby gems cache path for iOS monitoring.
The monitoring step checks app/vendor/bundle but iOS builds cache Ruby gems at app/ios/vendor/bundle (as configured in line 359 and confirmed by learnings). This inconsistency means the cache size won't be reported correctly for iOS builds.
Based on learnings
Apply this diff:
- 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
In .github/workflows/mobile-deploy.yml around lines 791-793 the monitoring step
checks "${{ env.APP_PATH }}/vendor/bundle" but iOS builds cache gems at "${{
env.APP_PATH }}/ios/vendor/bundle"; update the check to look for the iOS path
(or implement a small conditional that checks "${{ env.APP_PATH
}}/ios/vendor/bundle" first and falls back to "${{ env.APP_PATH
}}/vendor/bundle"), then compute GEMS_SIZE from the existing directory and echo
the result so the iOS cache size is reported correctly.
| - name: Apply version bump from outputs | ||
| run: | | ||
| cd ${{ env.APP_PATH }} | ||
| # Get current version from package.json | ||
| CURRENT_VERSION=$(node -p "require('./package.json').version") | ||
| # Get new version from version.json (if it exists and has version field) | ||
| if [ -f version.json ] && grep -q '"version"' version.json; then | ||
| NEW_VERSION=$(node -pe 'require("./version.json").version' 2>/dev/null || echo "") | ||
| else | ||
| # Fallback: use current version from package.json | ||
| NEW_VERSION="$CURRENT_VERSION" | ||
| 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'); | ||
| " | ||
| echo "✅ Versions applied successfully" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use version-manager script instead of duplicating logic.
The inline Node.js code duplicates the logic already implemented in the version-manager.cjs apply command. This creates a maintenance burden—if the version management logic changes, both places must be updated.
Replace the inline code with a call to 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');
- "
-
- echo "✅ Versions applied successfully"
+ # Use version-manager script to apply versions
+ node scripts/version-manager.cjs apply "$VERSION" "$IOS_BUILD" "$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.
| - name: Apply version bump from outputs | |
| run: | | |
| cd ${{ env.APP_PATH }} | |
| # Get current version from package.json | |
| CURRENT_VERSION=$(node -p "require('./package.json').version") | |
| # Get new version from version.json (if it exists and has version field) | |
| if [ -f version.json ] && grep -q '"version"' version.json; then | |
| NEW_VERSION=$(node -pe 'require("./version.json").version' 2>/dev/null || echo "") | |
| else | |
| # Fallback: use current version from package.json | |
| NEW_VERSION="$CURRENT_VERSION" | |
| 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'); | |
| " | |
| echo "✅ Versions applied successfully" | |
| - 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)" | |
| # Use version-manager script to apply versions | |
| node scripts/version-manager.cjs apply "$VERSION" "$IOS_BUILD" "$ANDROID_BUILD" |
🤖 Prompt for AI Agents
In .github/workflows/mobile-deploy.yml around lines 1250 to 1279, the inline
Node.js blocks duplicate version-management logic; replace them with a single
call to the existing version-manager script. Change the step to cd to ${{
env.APP_PATH }} and invoke the version-manager command (node
./path/to/version-manager.cjs apply or the repository's equivalent) passing the
VERSION, IOS_BUILD and ANDROID_BUILD values (or exporting them to the script) so
it updates package.json and version.json; ensure the step fails on non-zero exit
from the script and prints a success message on completion.
| function writePackageJson(data) { | ||
| try { | ||
| fs.writeFileSync(PACKAGE_JSON_PATH, JSON.stringify(data, null, 2) + '\n'); | ||
| } catch (error) { | ||
| throw new Error(`Failed to write package.json: ${error.message}`); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Write version.json | ||
| */ | ||
| function writeVersionJson(data) { | ||
| try { | ||
| fs.writeFileSync(VERSION_JSON_PATH, JSON.stringify(data, null, 2) + '\n'); | ||
| } catch (error) { | ||
| throw new Error(`Failed to write version.json: ${error.message}`); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add input validation before writing files.
The write functions accept any data without validation, which could corrupt the version files if invalid data is passed. This is especially risky since these files are the source of truth for deployments.
Consider adding validation helpers:
+/**
+ * Validate version string format (X.Y.Z)
+ */
+function isValidVersion(version) {
+ return /^\d+\.\d+\.\d+$/.test(version);
+}
+
+/**
+ * Validate build number (positive integer)
+ */
+function isValidBuild(build) {
+ return Number.isInteger(build) && build > 0;
+}
+
/**
* Write package.json
*/
function writePackageJson(data) {
+ if (!data.version || !isValidVersion(data.version)) {
+ throw new Error(`Invalid version format: ${data.version}. Expected X.Y.Z`);
+ }
try {
fs.writeFileSync(PACKAGE_JSON_PATH, JSON.stringify(data, null, 2) + '\n');
} catch (error) {
throw new Error(`Failed to write package.json: ${error.message}`);
}
}
/**
* Write version.json
*/
function writeVersionJson(data) {
+ if (!data?.ios?.build || !isValidBuild(data.ios.build)) {
+ throw new Error(`Invalid iOS build number: ${data?.ios?.build}`);
+ }
+ if (!data?.android?.build || !isValidBuild(data.android.build)) {
+ throw new Error(`Invalid Android build number: ${data?.android?.build}`);
+ }
try {
fs.writeFileSync(VERSION_JSON_PATH, JSON.stringify(data, null, 2) + '\n');
} catch (error) {
throw new Error(`Failed to write version.json: ${error.message}`);
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/scripts/version-manager.cjs around lines 67 to 84, both writePackageJson
and writeVersionJson accept any input and write it straight to disk; add input
validation before writing by implementing small helpers that verify the data is
the expected type (object), contains required keys (for package.json: at minimum
name and version; for version.json: at minimum version and build/timestamp or
whatever project expects), and that version strings match SemVer (or your
project's version format); if validation fails, throw a clear error and do not
write the file; optionally run JSON.stringify on a sanitized/normalized object
(ensuring no functions/undefined) to guarantee deterministic output.
| function getVersionInfo() { | ||
| const pkg = readPackageJson(); | ||
| const versionData = readVersionJson(); | ||
|
|
||
| return { | ||
| version: pkg.version, | ||
| iosBuild: versionData.ios.build, | ||
| androidBuild: versionData.android.build, | ||
| iosLastDeployed: versionData.ios.lastDeployed, | ||
| androidLastDeployed: versionData.android.lastDeployed, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Validate JSON structure before accessing nested properties.
The function directly accesses versionData.ios.build and versionData.android.build without verifying the structure exists, which will produce cryptic errors like "Cannot read property 'build' of undefined" if the JSON is malformed.
Apply this diff to add structure validation:
function getVersionInfo() {
const pkg = readPackageJson();
const versionData = readVersionJson();
+ // Validate structure
+ if (!pkg.version) {
+ throw new Error('package.json missing version field');
+ }
+ if (!versionData?.ios?.build || !versionData?.android?.build) {
+ throw new Error('version.json missing required ios.build or android.build fields');
+ }
+
return {
version: pkg.version,
iosBuild: versionData.ios.build,
androidBuild: versionData.android.build,
iosLastDeployed: versionData.ios.lastDeployed,
androidLastDeployed: versionData.android.lastDeployed,
};
}📝 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.
| function getVersionInfo() { | |
| const pkg = readPackageJson(); | |
| const versionData = readVersionJson(); | |
| return { | |
| version: pkg.version, | |
| iosBuild: versionData.ios.build, | |
| androidBuild: versionData.android.build, | |
| iosLastDeployed: versionData.ios.lastDeployed, | |
| androidLastDeployed: versionData.android.lastDeployed, | |
| }; | |
| } | |
| function getVersionInfo() { | |
| const pkg = readPackageJson(); | |
| const versionData = readVersionJson(); | |
| // Validate structure | |
| if (!pkg.version) { | |
| throw new Error('package.json missing version field'); | |
| } | |
| if (!versionData?.ios?.build || !versionData?.android?.build) { | |
| throw new Error('version.json missing required ios.build or android.build fields'); | |
| } | |
| return { | |
| version: pkg.version, | |
| iosBuild: versionData.ios.build, | |
| androidBuild: versionData.android.build, | |
| iosLastDeployed: versionData.ios.lastDeployed, | |
| androidLastDeployed: versionData.android.lastDeployed, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In app/scripts/version-manager.cjs around lines 89 to 100, getVersionInfo
currently accesses versionData.ios.build and versionData.android.build directly;
add explicit validation of versionData and its ios/android objects before
reading nested properties and return safe defaults or throw a descriptive error
if the structure is invalid. Modify the function to (a) check that versionData
is an object, (b) check that versionData.ios and versionData.android exist and
are objects, and (c) read .build and .lastDeployed only after those checks (or
use optional chaining and default to null/undefined), and if validation fails,
throw or log a clear error mentioning malformed version JSON so callers don’t
receive cryptic "cannot read property" exceptions.
| function bumpVersion(bumpType, platform = 'both') { | ||
| const validBumpTypes = ['major', 'minor', 'patch', 'build']; | ||
| const validPlatforms = ['ios', 'android', 'both']; | ||
|
|
||
| if (!validBumpTypes.includes(bumpType)) { | ||
| throw new Error( | ||
| `Invalid bump type: ${bumpType}. Expected: ${validBumpTypes.join(', ')}`, | ||
| ); | ||
| } | ||
|
|
||
| if (!validPlatforms.includes(platform)) { | ||
| throw new Error( | ||
| `Invalid platform: ${platform}. Expected: ${validPlatforms.join(', ')}`, | ||
| ); | ||
| } | ||
|
|
||
| const pkg = readPackageJson(); | ||
| const versionData = readVersionJson(); | ||
|
|
||
| let newVersion = pkg.version; | ||
|
|
||
| // Bump semantic version if major/minor/patch | ||
| if (bumpType !== 'build') { | ||
| newVersion = bumpSemanticVersion(pkg.version, bumpType); | ||
| console.log( | ||
| `📦 Bumping ${bumpType} version: ${pkg.version} → ${newVersion}`, | ||
| ); | ||
| } else { | ||
| console.log(`📦 Keeping version: ${newVersion} (build-only bump)`); | ||
| } | ||
|
|
||
| // Bump build numbers based on platform | ||
| let newIosBuild = versionData.ios.build; | ||
| let newAndroidBuild = versionData.android.build; | ||
|
|
||
| if (platform === 'ios' || platform === 'both') { | ||
| newIosBuild += 1; | ||
| console.log(`🍎 iOS build: ${versionData.ios.build} → ${newIosBuild}`); | ||
| } else { | ||
| console.log(`🍎 iOS build: ${newIosBuild} (unchanged)`); | ||
| } | ||
|
|
||
| if (platform === 'android' || platform === 'both') { | ||
| newAndroidBuild += 1; | ||
| console.log( | ||
| `🤖 Android build: ${versionData.android.build} → ${newAndroidBuild}`, | ||
| ); | ||
| } else { | ||
| console.log(`🤖 Android build: ${newAndroidBuild} (unchanged)`); | ||
| } | ||
|
|
||
| return { | ||
| version: newVersion, | ||
| iosBuild: newIosBuild, | ||
| androidBuild: newAndroidBuild, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Validate build numbers are integers before arithmetic operations.
The function increments build numbers with += 1 without verifying they're numeric, which could produce NaN or unexpected results if the JSON is corrupted with string values like "180" or invalid data.
Add validation at the start of the function:
function bumpVersion(bumpType, platform = 'both') {
const validBumpTypes = ['major', 'minor', 'patch', 'build'];
const validPlatforms = ['ios', 'android', 'both'];
if (!validBumpTypes.includes(bumpType)) {
throw new Error(
`Invalid bump type: ${bumpType}. Expected: ${validBumpTypes.join(', ')}`,
);
}
if (!validPlatforms.includes(platform)) {
throw new Error(
`Invalid platform: ${platform}. Expected: ${validPlatforms.join(', ')}`,
);
}
const pkg = readPackageJson();
const versionData = readVersionJson();
+ // Validate build numbers are positive integers
+ if (!Number.isInteger(versionData.ios.build) || versionData.ios.build < 1) {
+ throw new Error(`Invalid iOS build number: ${versionData.ios.build}. Expected positive integer.`);
+ }
+ if (!Number.isInteger(versionData.android.build) || versionData.android.build < 1) {
+ throw new Error(`Invalid Android build number: ${versionData.android.build}. Expected positive integer.`);
+ }
+
let newVersion = pkg.version;
// Bump semantic version if major/minor/patch
if (bumpType !== 'build') {
newVersion = bumpSemanticVersion(pkg.version, bumpType);📝 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.
| function bumpVersion(bumpType, platform = 'both') { | |
| const validBumpTypes = ['major', 'minor', 'patch', 'build']; | |
| const validPlatforms = ['ios', 'android', 'both']; | |
| if (!validBumpTypes.includes(bumpType)) { | |
| throw new Error( | |
| `Invalid bump type: ${bumpType}. Expected: ${validBumpTypes.join(', ')}`, | |
| ); | |
| } | |
| if (!validPlatforms.includes(platform)) { | |
| throw new Error( | |
| `Invalid platform: ${platform}. Expected: ${validPlatforms.join(', ')}`, | |
| ); | |
| } | |
| const pkg = readPackageJson(); | |
| const versionData = readVersionJson(); | |
| let newVersion = pkg.version; | |
| // Bump semantic version if major/minor/patch | |
| if (bumpType !== 'build') { | |
| newVersion = bumpSemanticVersion(pkg.version, bumpType); | |
| console.log( | |
| `📦 Bumping ${bumpType} version: ${pkg.version} → ${newVersion}`, | |
| ); | |
| } else { | |
| console.log(`📦 Keeping version: ${newVersion} (build-only bump)`); | |
| } | |
| // Bump build numbers based on platform | |
| let newIosBuild = versionData.ios.build; | |
| let newAndroidBuild = versionData.android.build; | |
| if (platform === 'ios' || platform === 'both') { | |
| newIosBuild += 1; | |
| console.log(`🍎 iOS build: ${versionData.ios.build} → ${newIosBuild}`); | |
| } else { | |
| console.log(`🍎 iOS build: ${newIosBuild} (unchanged)`); | |
| } | |
| if (platform === 'android' || platform === 'both') { | |
| newAndroidBuild += 1; | |
| console.log( | |
| `🤖 Android build: ${versionData.android.build} → ${newAndroidBuild}`, | |
| ); | |
| } else { | |
| console.log(`🤖 Android build: ${newAndroidBuild} (unchanged)`); | |
| } | |
| return { | |
| version: newVersion, | |
| iosBuild: newIosBuild, | |
| androidBuild: newAndroidBuild, | |
| }; | |
| } | |
| function bumpVersion(bumpType, platform = 'both') { | |
| const validBumpTypes = ['major', 'minor', 'patch', 'build']; | |
| const validPlatforms = ['ios', 'android', 'both']; | |
| if (!validBumpTypes.includes(bumpType)) { | |
| throw new Error( | |
| `Invalid bump type: ${bumpType}. Expected: ${validBumpTypes.join(', ')}`, | |
| ); | |
| } | |
| if (!validPlatforms.includes(platform)) { | |
| throw new Error( | |
| `Invalid platform: ${platform}. Expected: ${validPlatforms.join(', ')}`, | |
| ); | |
| } | |
| const pkg = readPackageJson(); | |
| const versionData = readVersionJson(); | |
| // Validate build numbers are positive integers | |
| if (!Number.isInteger(versionData.ios.build) || versionData.ios.build < 1) { | |
| throw new Error( | |
| `Invalid iOS build number: ${versionData.ios.build}. Expected positive integer.` | |
| ); | |
| } | |
| if (!Number.isInteger(versionData.android.build) || versionData.android.build < 1) { | |
| throw new Error( | |
| `Invalid Android build number: ${versionData.android.build}. Expected positive integer.` | |
| ); | |
| } | |
| let newVersion = pkg.version; | |
| // Bump semantic version if major/minor/patch | |
| if (bumpType !== 'build') { | |
| newVersion = bumpSemanticVersion(pkg.version, bumpType); | |
| console.log( | |
| `📦 Bumping ${bumpType} version: ${pkg.version} → ${newVersion}`, | |
| ); | |
| } else { | |
| console.log(`📦 Keeping version: ${newVersion} (build-only bump)`); | |
| } | |
| // Bump build numbers based on platform | |
| let newIosBuild = versionData.ios.build; | |
| let newAndroidBuild = versionData.android.build; | |
| if (platform === 'ios' || platform === 'both') { | |
| newIosBuild += 1; | |
| console.log(`🍎 iOS build: ${versionData.ios.build} → ${newIosBuild}`); | |
| } else { | |
| console.log(`🍎 iOS build: ${newIosBuild} (unchanged)`); | |
| } | |
| if (platform === 'android' || platform === 'both') { | |
| newAndroidBuild += 1; | |
| console.log( | |
| `🤖 Android build: ${versionData.android.build} → ${newAndroidBuild}`, | |
| ); | |
| } else { | |
| console.log(`🤖 Android build: ${newAndroidBuild} (unchanged)`); | |
| } | |
| return { | |
| version: newVersion, | |
| iosBuild: newIosBuild, | |
| androidBuild: newAndroidBuild, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In app/scripts/version-manager.cjs around lines 145 to 201, the code increments
ios/android build numbers without validating they are numeric, which can produce
NaN if the JSON contains strings or invalid data; before doing arithmetic, read
versionData.ios.build and versionData.android.build, coerce each to an integer
(e.g., parseInt or Number) and verify Number.isInteger and >= 0, fallback to 0
or throw a clear Error if invalid, then perform the +1 operations on the
validated integers and use those validated values in logs and the returned
object.
| function applyVersions(version, iosBuild, androidBuild) { | ||
| console.log(`📝 Applying versions to files...`); | ||
| console.log(` Version: ${version}`); | ||
| console.log(` iOS Build: ${iosBuild}`); | ||
| console.log(` Android Build: ${androidBuild}`); | ||
|
|
||
| // Update package.json | ||
| const pkg = readPackageJson(); | ||
| pkg.version = version; | ||
| writePackageJson(pkg); | ||
| console.log(`✅ Updated package.json`); | ||
|
|
||
| // Update version.json | ||
| const versionData = readVersionJson(); | ||
| versionData.ios.build = iosBuild; | ||
| versionData.android.build = androidBuild; | ||
| writeVersionJson(versionData); | ||
| console.log(`✅ Updated version.json`); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Validate function parameters before applying changes.
The function accepts version and build parameters without validation, which could write invalid data to the files if called with incorrect arguments (e.g., from a buggy workflow step or malicious input).
Add parameter validation:
function applyVersions(version, iosBuild, androidBuild) {
+ // Validate inputs
+ if (typeof version !== 'string' || !/^\d+\.\d+\.\d+$/.test(version)) {
+ throw new Error(`Invalid version format: ${version}. Expected X.Y.Z`);
+ }
+ if (!Number.isInteger(iosBuild) || iosBuild < 1) {
+ throw new Error(`Invalid iOS build number: ${iosBuild}. Expected positive integer.`);
+ }
+ if (!Number.isInteger(androidBuild) || androidBuild < 1) {
+ throw new Error(`Invalid Android build number: ${androidBuild}. Expected positive integer.`);
+ }
+
console.log(`📝 Applying versions to files...`);
console.log(` Version: ${version}`);
console.log(` iOS Build: ${iosBuild}`);
console.log(` Android Build: ${androidBuild}`);📝 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.
| function applyVersions(version, iosBuild, androidBuild) { | |
| console.log(`📝 Applying versions to files...`); | |
| console.log(` Version: ${version}`); | |
| console.log(` iOS Build: ${iosBuild}`); | |
| console.log(` Android Build: ${androidBuild}`); | |
| // Update package.json | |
| const pkg = readPackageJson(); | |
| pkg.version = version; | |
| writePackageJson(pkg); | |
| console.log(`✅ Updated package.json`); | |
| // Update version.json | |
| const versionData = readVersionJson(); | |
| versionData.ios.build = iosBuild; | |
| versionData.android.build = androidBuild; | |
| writeVersionJson(versionData); | |
| console.log(`✅ Updated version.json`); | |
| } | |
| function applyVersions(version, iosBuild, androidBuild) { | |
| // Validate inputs | |
| if (typeof version !== 'string' || !/^\d+\.\d+\.\d+$/.test(version)) { | |
| throw new Error(`Invalid version format: ${version}. Expected X.Y.Z`); | |
| } | |
| if (!Number.isInteger(iosBuild) || iosBuild < 1) { | |
| throw new Error(`Invalid iOS build number: ${iosBuild}. Expected positive integer.`); | |
| } | |
| if (!Number.isInteger(androidBuild) || androidBuild < 1) { | |
| throw new Error(`Invalid Android build number: ${androidBuild}. Expected positive integer.`); | |
| } | |
| console.log(`📝 Applying versions to files...`); | |
| console.log(` Version: ${version}`); | |
| console.log(` iOS Build: ${iosBuild}`); | |
| console.log(` Android Build: ${androidBuild}`); | |
| // Update package.json | |
| const pkg = readPackageJson(); | |
| pkg.version = version; | |
| writePackageJson(pkg); | |
| console.log(`✅ Updated package.json`); | |
| // Update version.json | |
| const versionData = readVersionJson(); | |
| versionData.ios.build = iosBuild; | |
| versionData.android.build = androidBuild; | |
| writeVersionJson(versionData); | |
| console.log(`✅ Updated version.json`); | |
| } |
🤖 Prompt for AI Agents
In app/scripts/version-manager.cjs around lines 206 to 224, the applyVersions
function currently writes package.json and version.json without validating
inputs; add parameter validation at the top of the function to prevent invalid
writes: verify version is a non-empty string matching a semver pattern (e.g.
/^\d+\.\d+\.\d+(-[0-9A-Za-z-.]+)?$/) and that iosBuild and androidBuild are
present and are non-negative integers (or numeric strings that parse to
integers); if validation fails, log a clear error and throw or exit without
modifying any files; after validation, coerce build values to integers before
assigning them and optionally ensure versionData.ios and versionData.android
objects exist before setting build fields.
refine mobile deploy auto pr logic
remove env check
fix get-version
Summary by CodeRabbit
New Features
Chores
Refactor