Repository navigation
Fix iOS App Store bundle identity - #7806
lawrencecchen wants to merge 8 commits into
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 iOS App Store configuration now uses ChangesApp Store identity alignment
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
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}
|
|
||
| ( | ||
| export "${asc_env[@]}" | ||
| local app_json app_bundle_identifier |
There was a problem hiding this comment.
Local variables in subshell
High Severity
The new App Store Connect bundle-id check declares app_json and app_bundle_identifier with local inside a ( … ) subshell. In Bash, local is only valid inside a function body, so the production upload_app_store_with_asc path aborts before asc apps view or upload run.
Reviewed by Cursor Bugbot for commit f6bc3bb. Configure here.
There was a problem hiding this comment.
Rejected after reproduction: this subshell executes inside upload_app_store_with_asc, so local remains in a function context. The exact pattern succeeds under macOS /bin/bash 3.2.57 and Bash 5.3.9. No code change needed.
— Claude Code
Greptile SummaryThis PR updates the production iOS App Store identity. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (8): Last reviewed commit: "Require numeric App Store app IDs" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| IOS_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" | ||
|
|
||
| APP="${ASC_APP_ID:-${IOS_APPSTORE_APP_ID:-${IOS_APPSTORE_BUNDLE_ID:-com.cmuxterm.app}}}" | ||
| APP="${ASC_APP_ID:-${IOS_APPSTORE_APP_ID:-${IOS_APPSTORE_BUNDLE_ID:-com.cmux.app}}}" |
There was a problem hiding this comment.
When this script runs without ASC_APP_ID or IOS_APPSTORE_APP_ID, APP falls back to com.cmux.app, but the later asc ... --app "$APP" calls expect the numeric App Store Connect app id. That makes a local/default validation run fail at the first ASC command with an invalid app identifier instead of validating the production app.
There was a problem hiding this comment.
Fixed in 4e9abf6. Validation now requires ASC_APP_ID/IOS_APPSTORE_APP_ID or an explicit numeric --app value and fails before invoking asc when none is configured; the usage text now says app-id.
— Claude Code
| IOS_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" | ||
|
|
||
| APP="${ASC_APP_ID:-${IOS_APPSTORE_APP_ID:-${IOS_APPSTORE_BUNDLE_ID:-com.cmuxterm.app}}}" | ||
| APP="${ASC_APP_ID:-${IOS_APPSTORE_APP_ID:-}}" |
There was a problem hiding this comment.
This removes the default bundle-id fallback, but callers can still pass --app com.cmux.app or set ASC_APP_ID=com.cmux.app. The script only checks that APP is non-empty before passing it into the later asc ... --app "$APP" commands, so stale local invocations still fail late at the App Store Connect command instead of being rejected by the script. Add an early numeric app-id check, or explicitly resolve bundle ids before any ASC command is built.
There was a problem hiding this comment.
Fixed in the latest commit. The validation script now requires ^[0-9]+$ before constructing any asc command; both a missing ID and an explicit --app com.cmux.app fail immediately with script-owned diagnostics.
— Claude Code
| ( | ||
| export "${asc_env[@]}" | ||
| local app_json app_bundle_identifier | ||
| app_json="$(asc apps view --id "$ASC_APP_ID" --output json)" |
There was a problem hiding this comment.
The upload path uses ASC_APP_ID in asc apps view before checking that it is a numeric App Store app id. When a local run or repo variable still has ASC_APP_ID=com.cmux.app, this lane reaches the ASC command and then fails through the CLI or JSON parsing instead of stopping with the script-owned app-id error used by the validation script. Add the same numeric guard before this command and before asc builds upload.
| ( | ||
| export "${asc_env[@]}" | ||
| local app_json app_bundle_identifier | ||
| app_json="$(asc apps view --id "$ASC_APP_ID" --output json)" |
There was a problem hiding this comment.
ASC_APP_ID reach the ASC CLI before the script checks that it is a numeric App Store Connect app id. For example, ASC_APP_ID=com.cmux.app passes the existing non-empty check, then this new asc apps view --id "$ASC_APP_ID" call can fail through the CLI and JSON parsing instead of the script-owned diagnostic used by the release validation script. Add the numeric guard before the first ASC command in this path.
| app_json="$(asc apps view --id "$ASC_APP_ID" --output json)" | |
| if [[ ! "$ASC_APP_ID" =~ ^[0-9]+$ ]]; then | |
| echo "error: ASC_APP_ID must be numeric (got '$ASC_APP_ID')" >&2 | |
| exit 2 | |
| fi | |
| app_json="$(asc apps view --id "$ASC_APP_ID" --output json)" |
| ( | ||
| export "${asc_env[@]}" | ||
| local app_json app_bundle_identifier | ||
| app_json="$(asc apps view --id "$ASC_APP_ID" --output json)" |
There was a problem hiding this comment.
ASC_APP_ID=com.cmux.app to pass the existing non-empty check and reach asc apps view --id "$ASC_APP_ID". In that case the lane fails through the ASC command or JSON handling instead of stopping with the script-owned numeric app-id error used by the release validation script. Add the same numeric guard before this first ASC command so both the app-record check and asc builds upload only receive App Store Connect app ids.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 31c0e57. Configure here.
| ( | ||
| export "${asc_env[@]}" | ||
| local app_json app_bundle_identifier | ||
| app_json="$(asc apps view --id "$ASC_APP_ID" --output json)" |
There was a problem hiding this comment.
ASC_APP_ID is only checked for being non-empty before this asc apps view --id "$ASC_APP_ID" call. If a local run or repo variable still has ASC_APP_ID=com.cmux.app, the new app-record check sends that bundle id to the ASC CLI before the script can produce its own numeric app-id error. Add the same numeric guard used by the release validator before the first ASC command in this upload path.
![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 `@ios/scripts/verify-ipa-release-identity.sh`:
- Line 27: Resolve and normalize the relative IPA path before the subshell
changes directories, then use that resolved path in the unzip command. Update
the extraction logic near the existing IPA validation and die handling so both
relative and absolute values of IPA_PATH remain accessible under WORKDIR.
🪄 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: 69826806-0ee6-4ca3-9c83-5bbae271a697
📒 Files selected for processing (5)
ios/scripts/require-numeric-app-store-id.shios/scripts/upload-testflight.shios/scripts/validate-app-store-release.shios/scripts/verify-ipa-release-identity.shtests/test_ios_app_store_identity.py
| WORKDIR="$(mktemp -d)" | ||
| trap 'rm -rf "$WORKDIR"' EXIT | ||
|
|
||
| ( cd "$WORKDIR" && unzip -q "$IPA_PATH" ) || die "could not unzip IPA: $IPA_PATH" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve relative IPA paths during extraction.
Line 27 changes directories before resolving $IPA_PATH. A valid relative path passes Line 18 but then cannot be found under $WORKDIR, blocking release verification.
Proposed fix
-( cd "$WORKDIR" && unzip -q "$IPA_PATH" ) || die "could not unzip IPA: $IPA_PATH"
+unzip -q "$IPA_PATH" -d "$WORKDIR" || die "could not unzip IPA: $IPA_PATH"📝 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.
| ( cd "$WORKDIR" && unzip -q "$IPA_PATH" ) || die "could not unzip IPA: $IPA_PATH" | |
| unzip -q "$IPA_PATH" -d "$WORKDIR" || die "could not unzip IPA: $IPA_PATH" |
🤖 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/verify-ipa-release-identity.sh` at line 27, Resolve and normalize
the relative IPA path before the subshell changes directories, then use that
resolved path in the unzip command. Update the extraction logic near the
existing IPA validation and die handling so both relative and absolute values of
IPA_PATH remain accessible under WORKDIR.
|
Superseded by merged #7809, which ships the same App Store bundle identity correction with stronger end-to-end lane coverage. Closing this duplicate instead of resolving conflicts. |
This branch was successfully deployed
1 participant
Fix the production iOS lane to build and sign
com.cmux.app, matching App Store Connect app 6783338052.Add a live app-record bundle ID check before upload so a mismatched binary fails before delivery. Align the profile resolver, validation defaults, workflow summary, and App Store release docs.
Verification:
bash -non the upload, validation, and profile scriptscom.cmux.appcmux Distributionprofile validated as7WLXT3NR37.com.cmux.appwith production APNs and Sign in with AppleNo app runtime behavior changes.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Changes production iOS release identity, signing validation, and App Store upload targeting—mistakes could block releases or mis-route uploads, though gates are designed to fail closed rather than ship wrong artifacts.
Overview
Production iOS App Store signing and release tooling now default to
com.cmux.appinstead ofcom.cmuxterm.app, including the CI provisioning-profile installer,upload-testflight.shappstore lane, docs, and the production workflow summary (CI profile namecmux App Store CI).Fail-closed gates were added so a wrong binary cannot reach App Store Connect:
verify-ipa-release-identity.shruns on every export path and checks Info.plist bundle id plus signed/profileapplication-identifier; the appstore upload path requires a numeric ASC app id (require-numeric-app-store-id.sh) and compares the live ASC app record’sbundleIdto the built lane beforeasc builds upload. Archive bundle-id validation now fails when the identifier is missing, not only on mismatch.validate-app-store-release.shno longer falls back to a bundle id for--app; it enforces the numeric app id helper. APNsroutePolicytreatscom.cmux.appas production (legacycom.cmuxterm.appretained). CI runstests/test_ios_app_store_identity.pyfor the new gates.Reviewed by Cursor Bugbot for commit cc4e175. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Align the production iOS App Store identity to bundle id
com.cmux.appacross build/signing, CI, docs, and APNs. Add fail-closed gates that verify the final IPA and the App Store Connect app before upload.Bug Fixes
com.cmux.appin provisioning installer, upload lane, validation, and CI; renamed CI profile tocmux App Store CI; updated docs and workflow summary.com.cmux.appas production (keepscom.cmuxterm.appanddev.cmux.app.beta); enforced numeric ASC app id viarequire-numeric-app-store-id.shinupload-testflight.shandvalidate-app-store-release.sh(no bundle-id fallback), with CI coverage.New Features
verify-ipa-release-identity.sh(run byupload-testflight.sh) verifies the final IPA’s Info.plist bundle id, signedapplication-identifier, and embedded profile match7WLXT3NR37.com.cmux.app.bundleIddoesn’t match the built bundle id; tightened archive bundle-id validation when missing; added a CI test for the final IPA identity gate.Written for commit cc4e175. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
com.cmux.app(instead ofcom.cmuxterm.app).Tests
Documentation
com.cmux.app.