Repository navigation
Separate iOS beta and App Store versions - #7854
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR separates iOS beta and App Store marketing versions, updates version bumping and release scripts to use lane-specific values, aligns workflows and documentation, and expands end-to-end tests for beta and App Store archive identity and version propagation. ChangesiOS versioning and release lanes
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow
participant upload-testflight.sh
participant Shared.xcconfig
participant xcodebuild
participant IPA
Workflow->>upload-testflight.sh: start lane upload
upload-testflight.sh->>Shared.xcconfig: read lane marketing version
upload-testflight.sh->>xcodebuild: archive with lane version
xcodebuild->>IPA: write archive metadata
upload-testflight.sh->>IPA: verify CFBundleShortVersionString
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (23 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 78b8ad5. Configure here.
Greptile SummaryThis PR splits the iOS beta and App Store release versions. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (7): Last reviewed commit: "Separate iOS beta and App Store versions" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| elif [[ "${1:-}" == "major" ]]; then | ||
| NEW_MARKETING="$((MAJOR + 1)).0.0" | ||
| elif [[ "$1" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then | ||
| elif [[ "${1:-}" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then |
There was a problem hiding this comment.
When the version comes before the lane, such as ios/scripts/bump-ios-version.sh 1.0.1 --lane appstore, this parser keeps the default beta lane and treats 1.0.1 as the version. The trailing --lane appstore is ignored, so the beta key is updated while the App Store version stays unchanged.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ios-app-store.yml:
- Line 168: Change the VERSION extraction command in the workflow to use tail -n
1 instead of head -n 1, matching read_xcconfig_setting semantics and ensuring
the last CMUX_IOS_APPSTORE_MARKETING_VERSION entry is used.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1b2bd4e6-4fc6-45da-a08a-10941c3618a8
📒 Files selected for processing (12)
.github/workflows/ios-app-store.yml.github/workflows/ios-testflight.ymlios/AppStoreReview/README.mdios/AppStoreReview/metadata-screenshots-checklist.mdios/CHANGELOG.mdios/Config/Shared.xcconfigios/README.mdios/scripts/bump-ios-version.shios/scripts/set-testflight-notes.shios/scripts/upload-testflight.shios/scripts/validate-app-store-release.shtests/test_ios_appstore_lane_identity.py
78b8ad5 to
8d722c2
Compare
| if [[ "${1:-}" == "--lane" ]]; then | ||
| if [[ -z "${2:-}" ]]; then | ||
| echo "Usage: $0 [--lane beta|appstore] [version|patch|minor|major]" >&2 | ||
| exit 1 | ||
| fi | ||
| LANE="${2:-}" | ||
| shift 2 | ||
| elif [[ "${1:-}" == --lane=* ]]; then | ||
| LANE="${1#--lane=}" | ||
| shift | ||
| fi |
There was a problem hiding this comment.
The parser still only recognizes --lane when it is the first argument. When a developer runs ios/scripts/bump-ios-version.sh 1.0.1 --lane appstore, this block is skipped, LANE stays beta, and the script updates CMUX_IOS_BETA_MARKETING_VERSION while leaving CMUX_IOS_APPSTORE_MARKETING_VERSION unchanged. That can make the next App Store upload use the old production version even though the bump command appeared to include the App Store lane.
8d722c2 to
a6c00d6
Compare
| if [[ "${1:-}" == "--lane" ]]; then | ||
| if [[ -z "${2:-}" ]]; then | ||
| echo "Usage: $0 [--lane beta|appstore] [version|patch|minor|major]" >&2 | ||
| exit 1 | ||
| fi | ||
| LANE="${2:-}" | ||
| shift 2 | ||
| elif [[ "${1:-}" == --lane=* ]]; then | ||
| LANE="${1#--lane=}" | ||
| shift |
There was a problem hiding this comment.
This parser only reads --lane when it is the first argument. When someone runs ios/scripts/bump-ios-version.sh 1.0.1 --lane appstore, LANE stays beta, 1.0.1 is accepted as the version, and the trailing lane flag is ignored. That updates CMUX_IOS_BETA_MARKETING_VERSION while the App Store version stays stale, so the next App Store release can still use the old production version.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/ios-app-store.yml (1)
168-168: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUse
tail -n 1to matchread_xcconfig_settingsemantics.The workflow extracts
CMUX_IOS_APPSTORE_MARKETING_VERSIONwithhead -n 1(first match), whilevalidate-app-store-release.sh'sread_xcconfig_settingusestail -n 1(last match). In xcconfig, the last value wins, sotail -n 1is correct. If the xcconfig ever has duplicate entries, the workflow would pass the wrong version to the validation script.🔧 Proposed fix
- VERSION="$(sed -nE 's/^[[:space:]]*CMUX_IOS_APPSTORE_MARKETING_VERSION[[:space:]]*=[[:space:]]*([^[:space:]]+).*/\1/p' ios/Config/Shared.xcconfig | head -n 1)" + VERSION="$(sed -nE 's/^[[:space:]]*CMUX_IOS_APPSTORE_MARKETING_VERSION[[:space:]]*=[[:space:]]*([^[:space:]]+).*/\1/p' ios/Config/Shared.xcconfig | tail -n 1)"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ios-app-store.yml at line 168, Change the VERSION extraction command in the workflow to use `tail -n 1` instead of `head -n 1`, matching the last-value-wins behavior implemented by `read_xcconfig_setting` in `validate-app-store-release.sh`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ios/scripts/bump-ios-version.sh`:
- Line 48: Replace the exact-space grep parsing in the version reads of the bump
script, including CURRENT_MARKETING and the re-read around line 83, with the
existing read_xcconfig_setting pattern used by the other scripts. Reuse or
define that helper to tolerate arbitrary whitespace around the xcconfig key and
equals sign, and use it consistently for both reads.
---
Duplicate comments:
In @.github/workflows/ios-app-store.yml:
- Line 168: Change the VERSION extraction command in the workflow to use `tail
-n 1` instead of `head -n 1`, matching the last-value-wins behavior implemented
by `read_xcconfig_setting` in `validate-app-store-release.sh`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: fbe55308-2d6e-4188-a3b6-b873fa0cc123
📒 Files selected for processing (13)
.github/workflows/ios-app-store.yml.github/workflows/ios-testflight.ymlios/AppStoreReview/README.mdios/AppStoreReview/metadata-screenshots-checklist.mdios/CHANGELOG.mdios/Config/Shared.xcconfigios/README.mdios/scripts/bump-ios-version.shios/scripts/cloud-testflight.shios/scripts/set-testflight-notes.shios/scripts/upload-testflight.shios/scripts/validate-app-store-release.shtests/test_ios_appstore_lane_identity.py
52d2057 to
218856e
Compare
218856e to
6014d8a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ios/scripts/validate-app-store-release.sh`:
- Around line 49-54: Extract read_xcconfig_setting into a shared
ios/scripts/xcconfig-helpers.sh helper, preserving its current parsing behavior,
and source that helper from validate-app-store-release.sh and
upload-testflight.sh. Replace the duplicated inline sed parsing in the iOS
workflow with the shared helper or an equivalent invocation so all xcconfig
extraction uses one implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bb4743d4-60dd-4418-be9e-e8d71ce4e8f1
📒 Files selected for processing (13)
.github/workflows/ios-app-store.yml.github/workflows/ios-testflight.ymlios/AppStoreReview/README.mdios/AppStoreReview/metadata-screenshots-checklist.mdios/CHANGELOG.mdios/Config/Shared.xcconfigios/README.mdios/scripts/bump-ios-version.shios/scripts/cloud-testflight.shios/scripts/set-testflight-notes.shios/scripts/upload-testflight.shios/scripts/validate-app-store-release.shtests/test_ios_appstore_lane_identity.py
| read_xcconfig_setting() { | ||
| local key="$1" | ||
| local file="$2" | ||
| sed -nE "s/^[[:space:]]*$key[[:space:]]*=[[:space:]]*([^[:space:]]+).*/\\1/p" "$file" 2>/dev/null | tail -n 1 | ||
| } | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Consider extracting read_xcconfig_setting into a shared helper.
The same read_xcconfig_setting function now exists in validate-app-store-release.sh and upload-testflight.sh, and the same sed logic is inlined in .github/workflows/ios-app-store.yml line 168. Duplicating the extraction logic risks divergence if the xcconfig format or parsing semantics change. Consider sourcing a shared ios/scripts/xcconfig-helpers.sh from both scripts.
🧰 Tools
🪛 Shellcheck (0.11.0)
[error] 52-52: Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet).
(SC1087)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ios/scripts/validate-app-store-release.sh` around lines 49 - 54, Extract
read_xcconfig_setting into a shared ios/scripts/xcconfig-helpers.sh helper,
preserving its current parsing behavior, and source that helper from
validate-app-store-release.sh and upload-testflight.sh. Replace the duplicated
inline sed parsing in the iOS workflow with the shared helper or an equivalent
invocation so all xcconfig extraction uses one implementation.
This branch was successfully deployed
1 participant
Summary
Testing
Notes
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Changes release/version stamping and pre-export guards on TestFlight and App Store upload paths; mistakes could block uploads or ship the wrong CFBundleShortVersionString, but scope is tooling and config with expanded automated tests.
Overview
Introduces independent TestFlight beta and production App Store marketing versions so beta can stay on an already-approved line (e.g.
1.0.4) while the first App Store upload can ship as1.0.0.ios/Config/Shared.xcconfignow definesCMUX_IOS_BETA_MARKETING_VERSIONandCMUX_IOS_APPSTORE_MARKETING_VERSION, withMARKETING_VERSIONaliasing the beta value for normal Xcode builds.upload-testflight.shpicks the lane version, stamps it at archive time, and fails closed if a reused archive’sCFBundleShortVersionStringdoes not match. Beta--auto-versionfloors on the greater of the latestios-v*tag and the checked-in beta version.bump-ios-version.shgains--lane beta|appstore;validate-app-store-release.shand the App Store workflow default to the App Store key.cloud-testflight.shpinsBETA_MARKETING_VERSIONbefore fleet/local archives. Docs, changelog wording, andtest_ios_appstore_lane_identity.pyare updated for lane-specific stamping and validation.Reviewed by Cursor Bugbot for commit 6014d8a. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Split iOS beta and App Store marketing versions and make each lane stamp and enforce its own
MARKETING_VERSIONat archive time. Prevents wrong-version uploads and keeps beta and production release lines independent.New Features
CMUX_IOS_BETA_MARKETING_VERSIONandCMUX_IOS_APPSTORE_MARKETING_VERSIONinios/Config/Shared.xcconfig;MARKETING_VERSIONdefaults to the beta value.CFBundleShortVersionStringis invalid or mismatched; final IPAs keep the lane’s version.--auto-version(beta-only) sets the next patch over the greater of the latestios-vX.Y.Ztag or the checked-in beta version; never decreases and fails closed if it can’t compute a version. App Store lane rejects--auto-version.cloud-testflight.shpinsBETA_MARKETING_VERSIONearly;validate-app-store-release.shand the App Store workflow default toCMUX_IOS_APPSTORE_MARKETING_VERSION;bump-ios-version.sh --lane beta|appstoreupdates the targeted lane and accepts trailing--lane; workflows/docs reference the new keys. Tests cover lane stamping in archives/IPAs, archive guards, auto-version flooring, validation defaults, and the bump helper.Migration
ios/scripts/bump-ios-version.sh --lane beta|appstoreto bump the correct lane.ios/CHANGELOG.mdentry equal toCMUX_IOS_BETA_MARKETING_VERSION. No runtime changes.Written for commit 6014d8a. Summary will update on new commits.
Summary by CodeRabbit