Skip to content

cloud-testflight: --marketing-version override so uploads outrank installed builds - #5858

Merged
lawrencecchen merged 3 commits into
mainfrom
feat-beta-version-bump
Jun 12, 2026
Merged

lawrencecchen merged 3 commits into
mainfrom
feat-beta-version-bump

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Jun 11, 2026 •

Copy link
Copy Markdown
Contributor

TestFlight orders builds by marketing version before build number. Today's beta cuts archived with the xcconfig default (1.0.0) while testers run 1.0.1 (20260609105221), so the new builds were never offered as updates. Adds an explicit --marketing-version flag (env IOS_BETA_MARKETING_VERSION), plumbed through both the cloud lane (BETA_MARKETING_VERSION to the hq reload-cloud-ios beta-archive xcodebuild, hq side already landed) and the local archive fallback.

🤖 Generated with Claude Code


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Note

Medium Risk
Touches the production beta TestFlight pipeline (version ordering and signed entitlements); mistakes could hide updates or drop capabilities until profiles are regenerated.

Overview
Adds --marketing-version (and IOS_BETA_MARKETING_VERSION) to cloud-testflight.sh so beta cuts can set MARKETING_VERSION above what testers already have installed—TestFlight ranks by marketing version first, so lower versions never show as updates. The value is validated up front, passed into local xcodebuild, and exported as BETA_MARKETING_VERSION for fleet builds only when set (explicitly unset otherwise so env cannot leak).

In upload-testflight.sh, manual re-sign no longer merges only export baseline + Release entitlements. It now merges the embedded provisioning profile’s Entitlements, then Release, then drops any key the profile does not authorize (with stderr warnings). That fixes ASC 90163 rejections and restores profile-only keys like keychain-access-groups that a Release-only merge omitted.

Reviewed by Cursor Bugbot for commit 7f93496. Bugbot is set up for automated code reviews on this repo. Configure here.


Summary by cubic

Adds a --marketing-version override so TestFlight uploads outrank installed builds and show as updates. Also fixes manual re-signing to align entitlements with the provisioning profile, preventing ASC 90163 and keeping required profile-only keys.

  • New Features

    • ios/scripts/cloud-testflight.sh: --marketing-version <X.Y[.Z]> (defaults from IOS_BETA_MARKETING_VERSION), passed as MARKETING_VERSION locally and BETA_MARKETING_VERSION in cloud; validates format early and explicitly unsets BETA_MARKETING_VERSION when not provided.
  • Bug Fixes

    • ios/scripts/upload-testflight.sh: during manual re-sign, seed entitlements from the embedded profile, merge Release, then drop any key not authorized by the profile with warnings; fixes ASC 90163 and preserves profile-only keys like keychain-access-groups.

Written for commit 7f93496. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Chores
    • TestFlight build process now supports overriding the iOS marketing version to control update ordering via a new CLI option for cloud and local builds; the override is validated and will not leak a caller environment value into cloud builds.
  • Bug Fixes
    • Manual re-signing for TestFlight uploads now reconciles app entitlements with the provisioning profile, removing unauthorized keys and warning for dropped keys to prevent upload rejections.

@vercel

vercel Bot commented Jun 11, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Jun 12, 2026 2:04am
cmux-staging Building Building Preview, Comment Jun 12, 2026 2:04am

@coderabbitai

coderabbitai Bot commented Jun 11, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a --marketing-version option and MARKETING_VERSION_OVERRIDE for cloud/local TestFlight builds (with format validation and explicit unset); and updates manual re-sign to seed merged entitlements from the provisioning profile, merge Release entitlements, and drop keys unauthorized by the profile.

Changes

TestFlight Marketing Version Override

Layer / File(s) Summary
CLI interface and variable declaration
ios/scripts/cloud-testflight.sh
Introduces MARKETING_VERSION_OVERRIDE initialized from IOS_BETA_MARKETING_VERSION and documents the new --marketing-version <X.Y.Z> usage.
Argument parsing for marketing version option
ios/scripts/cloud-testflight.sh
Extends argument parsing to accept --marketing-version and store its value in MARKETING_VERSION_OVERRIDE.
Validation and build path integration
ios/scripts/cloud-testflight.sh
Validates override format (X.Y or X.Y.Z); exports BETA_MARKETING_VERSION to the cloud beta-archive flow when provided (or explicitly unsets it), and conditionally injects MARKETING_VERSION="..." into local xcodebuild when override is set.

Manual Re-sign Entitlements Reconciliation

Layer / File(s) Summary
Documentation and profile behavior
ios/scripts/upload-testflight.sh
Expands in-script docs describing ASC rejection risk for unauthorized entitlement keys and notes that dropped capabilities require regenerating the provisioning profile snapshot.
Entitlements extraction, merge, and prune
ios/scripts/upload-testflight.sh
Seed merged entitlements from the provisioning profile’s Entitlements (decoded from embedded.mobileprovision), merge profile then Release entitlements into MERGED_ENTITLEMENTS, and remove any entitlement keys not authorized by the profile while emitting warnings.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • manaflow-ai/cmux#5448: Modifies ios/scripts/upload-testflight.sh’s signing/export behavior and touches related manual signing flows.

Poem

I'm a rabbit in a TestFlight race 🐇
Version numbers hopping into place,
Entitlements pruned with careful art,
Uploads tidy, each key a part,
I nibble carrots and cheer the chart.


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 2 warnings)

Check name Status Explanation Resolution
Cmux User-Facing Error Privacy ❌ Error PR adds a user-facing warning in ios/scripts/upload-testflight.sh that includes upstream service name “ASC” and raw error code “90163”, violating user-facing error privacy rules. Sanitize the stderr warning to omit “ASC” and “90163” (e.g., “provisioning profile does not authorize; dropping entitlement {key}”) while keeping any raw upstream details out of user-visible output.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The PR description covers the problem (TestFlight version ordering), solution (--marketing-version flag and entitlements fix), and includes implementation details. However, it lacks required sections for Testing and Checklist compliance per the template. Add a Testing section describing how the changes were tested (local verification, cloud build validation) and complete the Checklist with checkmarks for testing, documentation updates, and bot review requests.
✅ Passed checks (18 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding a --marketing-version override to ensure TestFlight uploads rank higher than installed builds.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed PR #5858 modifies only ios/scripts/cloud-testflight.sh and ios/scripts/upload-testflight.sh; no Swift production code changes, so it can’t introduce Swift 6 actor-isolation mistakes.
Cmux Swift Blocking Runtime ✅ Passed PR #5858 changes only ios/scripts/cloud-testflight.sh and ios/scripts/upload-testflight.sh (no Swift diffs), so it can’t introduce blocking/timing sync in production Swift.
Cmux Expensive Synchronous Load ✅ Passed Detected PR-specific logic only in ios/scripts/cloud-testflight.sh and ios/scripts/upload-testflight.sh (e.g., MARKETING_VERSION_OVERRIDE, MERGED_ENTITLEMENTS); no production Swift loader placement...
Cmux Cache Substitution Correctness ✅ Passed PR #5858 changes only ios/scripts/*.sh (cloud-testflight.sh, upload-testflight.sh); no production Swift/TypeScript/JavaScript diffs touching persistence/history/snapshot cache substitution.
Cmux No Hacky Sleeps ✅ Passed PR #5858 only changes marketing-version override plumbing in cloud-testflight.sh and entitlement merge/dropping in upload-testflight.sh; no diff introduces sleep/usleep/setTimeout/setInterval/timer...
Cmux Algorithmic Complexity ✅ Passed cloud-testflight.sh adds constant-time flag parsing/regex validation and env unset/export; upload-testflight.sh drops entitlement keys via one dict-membership scan (no nested full scans).
Cmux Swift Concurrency ✅ Passed PR #5858 diff changes only ios/scripts/cloud-testflight.sh and upload-testflight.sh (no .swift matches), so no legacy Swift concurrency patterns were introduced or expanded.
Cmux Swift @Concurrent ✅ Passed PR #5858 changes only ios/scripts/*.sh (no Swift files/code), so the swift-concurrent-annotation rules are not applicable.
Cmux Swift File And Package Boundaries ✅ Passed PR #5858 changes only ios/scripts/cloud-testflight.sh and ios/scripts/upload-testflight.sh (no production .swift diffs), so no Swift file/package-boundary violations can occur.
Cmux Swift Logging ✅ Passed PR #5858’s “Files changed” view shows only ios/scripts/cloud-testflight.sh and ios/scripts/upload-testflight.sh; no Swift app/runtime code was added/changed, so swift-logging rules are not violated.
Cmux Full Internationalization ✅ Passed PR #5858 only modifies ios/scripts/cloud-testflight.sh and ios/scripts/upload-testflight.sh; no Swift/web/user-facing localized strings or locale catalogs are touched, so the full-internationalizat...
Cmux Swiftui State Layout ✅ Passed PR #5858 only changes ios/scripts/cloud-testflight.sh and ios/scripts/upload-testflight.sh (no Swift/SwiftUI diffs), so swiftui-state-layout.md rules don’t apply.
Cmux Architecture Rethink ✅ Passed PR 5858 changes only iOS build/upload shell scripts (cloud-testflight.sh, upload-testflight.sh); no Swift/SwiftUI architecture code is modified, so swift-architectural-rethink rules don’t apply.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed Repo changes are confined to iOS TestFlight tooling scripts (cloud-testflight.sh, upload-testflight.sh); no Swift auxiliary window/close-shortcut code appears to be modified, so the rule isn’t trig...
Cmux Source Artifacts ✅ Passed PR changes only ios/scripts/cloud-testflight.sh and ios/scripts/upload-testflight.sh; no added temp/DerivedData/cache/log/artifact paths that the source-control-artifacts rule forbids.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-beta-version-bump

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Jun 11, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds a --marketing-version override to cloud-testflight.sh so uploads carry a version number above what testers have installed (fixing invisible builds in TestFlight). Also hardens the manual re-signing path in upload-testflight.sh by seeding entitlements from the embedded provisioning profile before merging the Release file, then intersecting the result against the profile to drop unauthorized keys — fixing ASC upload error 90163.

  • cloud-testflight.sh: New --marketing-version <X.Y[.Z]> flag (also read from IOS_BETA_MARKETING_VERSION), validated eagerly before any build starts; passed as BETA_MARKETING_VERSION env var to the cloud fleet and as MARKETING_VERSION= build setting to the local xcodebuild archive fallback.
  • upload-testflight.sh: Re-sign now (1) decodes embedded.mobileprovision, (2) merges the profile's Entitlements dict into the working plist so profile-only keys like keychain-access-groups are preserved, (3) runs a Python script to drop any key the profile does not authorize, warning per dropped key so the cut log is auditable.

Confidence Score: 5/5

Safe to merge — the marketing-version plumbing is validated before any build starts, the entitlements reconciliation directly addresses the ASC 90163 rejection and the keychain-access-groups loss, and the hard post-sign gates (aps-environment check, strict codesign verify) remain in place.

Both changes are tightly scoped: the marketing-version flag adds a validated passthrough with no side effects on builds where it is not set, and the entitlements fix is an additive reconciliation step that preserves all existing gates. No production code outside the beta cut tooling is touched.

No files require special attention.

Important Files Changed

Filename Overview
ios/scripts/cloud-testflight.sh Adds --marketing-version / IOS_BETA_MARKETING_VERSION override with eager format validation (X.Y or X.Y.Z); plumbed to cloud via BETA_MARKETING_VERSION export and to local archive via MARKETING_VERSION xcodebuild arg. Usage help text is missing the new flag's description entry.
ios/scripts/upload-testflight.sh Entitlements reconciliation reworked: seeds from the embedded provisioning profile's Entitlements dict (adds profile-only keys like keychain-access-groups), merges Release entitlements, then uses a Python script to drop any key not authorized by the profile (fixes ASC error 90163). plutil -extract is missing an explicit error handler.

Sequence Diagram

sequenceDiagram
    participant User
    participant cloud-testflight.sh
    participant reload-cloud-ios.sh (hq)
    participant xcodebuild (local)
    participant upload-testflight.sh
    participant ASC

    User->>cloud-testflight.sh: --marketing-version 1.0.2
    cloud-testflight.sh->>cloud-testflight.sh: validate format

    alt Cloud path
        cloud-testflight.sh->>reload-cloud-ios.sh (hq): BETA_MARKETING_VERSION=1.0.2 (exported env)
        reload-cloud-ios.sh (hq)-->>cloud-testflight.sh: BETA_ARCHIVE_PATH=…
    else Local fallback
        cloud-testflight.sh->>xcodebuild (local): MARKETING_VERSION=1.0.2 (build setting arg)
        xcodebuild (local)-->>cloud-testflight.sh: .xcarchive
    end

    cloud-testflight.sh->>upload-testflight.sh: --archive-path …
    upload-testflight.sh->>upload-testflight.sh: export IPA
    upload-testflight.sh->>upload-testflight.sh: decode embedded.mobileprovision
    upload-testflight.sh->>upload-testflight.sh: PlistBuddy Merge profile entitlements
    upload-testflight.sh->>upload-testflight.sh: PlistBuddy Merge Release entitlements
    upload-testflight.sh->>upload-testflight.sh: python3: drop keys not in profile
    upload-testflight.sh->>upload-testflight.sh: "codesign + verify aps-environment=production"
    upload-testflight.sh->>ASC: altool upload
    ASC-->>User: build visible in TestFlight
Loading

Reviews (4): Last reviewed commit: "cloud-testflight: validate --marketing-v..." | Re-trigger Greptile

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
ios/scripts/cloud-testflight.sh (1)

72-99: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add detailed documentation for the --marketing-version flag.

The flag appears in the usage synopsis (line 72) but has no explanation in the detailed options section (lines 79-99). Users need to understand when and why to override the marketing version, especially the TestFlight ordering behavior mentioned in the PR description.

📝 Suggested documentation addition

Add after line 93 (the --local explanation):

   --local            Build the Release archive locally on this Mac instead of the
                      fleet. Used automatically when no hq cloud script is present.
+  --marketing-version <X.Y.Z>
+                     Override the iOS marketing version (CFBundleShortVersionString).
+                     TestFlight orders builds by marketing version FIRST, then build
+                     number; uploading below testers' installed version makes the
+                     build invisible as an update. Use when cutting a new beta after
+                     the xcconfig default has fallen behind installed builds.
   --keep-artifacts   Keep the downloaded archive + export dir.
🤖 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/cloud-testflight.sh` around lines 72 - 99, Add a detailed
description for the --marketing-version flag in ios/scripts/cloud-testflight.sh:
after the existing --local explanation, document that --marketing-version
<X.Y.Z> overrides the app’s CFBundleShortVersionString used by TestFlight/App
Store metadata (it does not change the bundle id or build number), explain the
allowed format (semantic X.Y.Z), and note when to use it (e.g., to control
TestFlight ordering when you need a visible higher/lower marketing version than
the built archive or to align versions across builds), plus any implications for
beta review and upload-testflight.sh behavior (mapping to the exported plist
value).
🤖 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/cloud-testflight.sh`:
- Line 105: When handling the --marketing-version flag, ensure you first verify
the next token exists and is not another flag before assigning
MARKETING_VERSION_OVERRIDE and shifting; if empty or starts with '-' print a
clear error and exit instead of shifting away the next flag. After extracting
MARKETING_VERSION_OVERRIDE, validate it against a semver pattern (e.g., require
X.Y.Z with digits and dots) and if it fails print an informative error and exit
(referencing the --marketing-version flag and MARKETING_VERSION_OVERRIDE
variable). Update the parsing block that currently sets
MARKETING_VERSION_OVERRIDE="${2:-}"; shift 2 to perform these presence and regex
checks and only shift when the value is valid.

---

Outside diff comments:
In `@ios/scripts/cloud-testflight.sh`:
- Around line 72-99: Add a detailed description for the --marketing-version flag
in ios/scripts/cloud-testflight.sh: after the existing --local explanation,
document that --marketing-version <X.Y.Z> overrides the app’s
CFBundleShortVersionString used by TestFlight/App Store metadata (it does not
change the bundle id or build number), explain the allowed format (semantic
X.Y.Z), and note when to use it (e.g., to control TestFlight ordering when you
need a visible higher/lower marketing version than the built archive or to align
versions across builds), plus any implications for beta review and
upload-testflight.sh behavior (mapping to the exported plist value).
🪄 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: 04e47edd-eae1-4922-86de-93cef5b90e6a

📥 Commits

Reviewing files that changed from the base of the PR and between 8fef9f6 and f606aac.

📒 Files selected for processing (1)
  • ios/scripts/cloud-testflight.sh

while [[ $# -gt 0 ]]; do
case "$1" in
--no-upload) NO_UPLOAD=1; shift ;;
--marketing-version) MARKETING_VERSION_OVERRIDE="${2:-}"; shift 2 ;;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Validate the --marketing-version argument value.

The current parsing has two issues:

  1. Missing empty-value check: If --marketing-version is passed without a value (e.g., --marketing-version --external), ${2:-} evaluates to empty, and shift 2 consumes the next flag (--external), causing confusing errors.

  2. No format validation: The script doesn't verify the version string is valid semver (e.g., X.Y.Z). Invalid values like "garbage" will be silently passed to xcodebuild, potentially causing build failures or incorrect metadata.

🛡️ Proposed validation fix
-    --marketing-version) MARKETING_VERSION_OVERRIDE="${2:-}"; shift 2 ;;
+    --marketing-version)
+      MARKETING_VERSION_OVERRIDE="${2:-}"
+      [[ -n "$MARKETING_VERSION_OVERRIDE" ]] || die "--marketing-version requires a version argument (e.g., 1.0.0)"
+      [[ "$MARKETING_VERSION_OVERRIDE" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]] || die "invalid marketing version format: $MARKETING_VERSION_OVERRIDE (expected X.Y.Z semver)"
+      shift 2
+      ;;
🤖 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/cloud-testflight.sh` at line 105, When handling the
--marketing-version flag, ensure you first verify the next token exists and is
not another flag before assigning MARKETING_VERSION_OVERRIDE and shifting; if
empty or starts with '-' print a clear error and exit instead of shifting away
the next flag. After extracting MARKETING_VERSION_OVERRIDE, validate it against
a semver pattern (e.g., require X.Y.Z with digits and dots) and if it fails
print an informative error and exit (referencing the --marketing-version flag
and MARKETING_VERSION_OVERRIDE variable). Update the parsing block that
currently sets MARKETING_VERSION_OVERRIDE="${2:-}"; shift 2 to perform these
presence and regex checks and only shift when the value is valid.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 594f2f1. Configure here.

Comment thread ios/scripts/cloud-testflight.sh Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 594f2f16b6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

while [[ $# -gt 0 ]]; do
case "$1" in
--no-upload) NO_UPLOAD=1; shift ;;
--marketing-version) MARKETING_VERSION_OVERRIDE="${2:-}"; shift 2 ;;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Stamp the workflow TestFlight archive too

In the production GitHub lane I checked, .github/workflows/ios-testflight.yml still invokes ./ios/scripts/upload-testflight.sh directly, and that script's archive command only overrides CURRENT_PROJECT_VERSION, leaving MARKETING_VERSION at ios/Config/Shared.xcconfig's default. This new flag/env only affects cloud-testflight.sh's cloud/local archive paths, so scheduled/manual TestFlight uploads from the workflow will still ship as 1.0.0 and remain hidden behind testers' installed 1.0.1 builds—the exact scenario this change is meant to prevent.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/upload-testflight.sh`:
- Around line 540-547: Replace the blind "|| true" around the two PlistBuddy
Merge invocations so genuine failures still abort: run /usr/libexec/PlistBuddy
-c "Merge $PROFILE_ENTITLEMENTS" "$MERGED_ENTITLEMENTS" and the same for
$RELEASE_ENTITLEMENTS while capturing stdout/stderr and the exit code; if exit
is non-zero, inspect the captured output and only suppress the error when it
contains the specific "Duplicate Entry Was Skipped" message, otherwise print the
PlistBuddy output and exit non‑zero so the script fails fast; update the merge
logic surrounding PROFILE_ENTITLEMENTS, RELEASE_ENTITLEMENTS and
MERGED_ENTITLEMENTS accordingly.
🪄 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: d608122a-8679-4d5a-b26a-ca80392f6c53

📥 Commits

Reviewing files that changed from the base of the PR and between f606aac and 594f2f1.

📒 Files selected for processing (1)
  • ios/scripts/upload-testflight.sh

Comment on lines +540 to 547
# `|| true`: PlistBuddy Merge prints "Duplicate Entry Was Skipped" if a
# source key overlaps an existing key. That is the intended behavior
# (existing wins), but its exit code on that path is not contractually 0 across
# OS versions, and a stray non-zero would kill the script under `set -e`. The
# exit code is non-load-bearing anyway: a genuinely failed merge produces no
# aps-environment and is caught by the hard gate below with a clear error.
/usr/libexec/PlistBuddy -c "Merge $PROFILE_ENTITLEMENTS" "$MERGED_ENTITLEMENTS" >/dev/null || true
/usr/libexec/PlistBuddy -c "Merge $RELEASE_ENTITLEMENTS" "$MERGED_ENTITLEMENTS" >/dev/null || true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n 'PlistBuddy -c "Merge .*" .*\\|\\| true' ios/scripts/upload-testflight.sh

Repository: manaflow-ai/cmux

Length of output: 2493


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the merge+gate region
echo "=== Context around Merge at ~520-610 ==="
sed -n '500,620p' ios/scripts/upload-testflight.sh | nl -ba | sed -n '1,160p'

echo
echo "=== Search for entitlement gate conditions ==="
rg -n --hidden --no-ignore-vcs 'aps-environment|aps_environment|entitlements|entitlement(s)?|hard gate|gating|MERGED_ENTITLEMENTS|PROFILE_ENTITLEMENTS|RELEASE_ENTITLEMENTS' ios/scripts/upload-testflight.sh

Repository: manaflow-ai/cmux

Length of output: 162


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Context around Merge at ~520-610 (with line numbers) ==="
# use awk to prefix line numbers
sed -n '500,620p' ios/scripts/upload-testflight.sh | awk '{printf "%6d  %s\n", NR+499, $0}'

echo
echo "=== Search for entitlement gate conditions (limited) ==="
rg -n --hidden --no-ignore-vcs 'aps-environment|aps_environment|MERGED_ENTITLEMENTS|PROFILE_ENTITLEMENTS|RELEASE_ENTITLEMENTS|gating|hard gate|entitlement(s)?|entitlements' ios/scripts/upload-testflight.sh | head -n 80

Repository: manaflow-ai/cmux

Length of output: 13562


Stop swallowing PlistBuddy merge failures in entitlements reconciliation.

|| true at lines 546-547 ignores all non-zero exits from PlistBuddy Merge, not just the OS-specific “Duplicate Entry Was Skipped” case. Since the hard gates that follow only validate aps-environment (presence + production), other entitlement reconciliation failures could still produce an IPA that passes upload while missing entitlements.

Proposed fix
+  merge_plist_allow_duplicates_only() {
+    local src="$1"
+    local dst="$2"
+    local err
+    if ! err=$(/usr/libexec/PlistBuddy -c "Merge '$src'" "$dst" 2>&1); then
+      if [[ "$err" != *"Duplicate Entry Was Skipped"* ]]; then
+        echo "error: entitlement merge failed for $src: $err" >&2
+        exit 1
+      fi
+    fi
+  }
-  /usr/libexec/PlistBuddy -c "Merge $PROFILE_ENTITLEMENTS" "$MERGED_ENTITLEMENTS" >/dev/null || true
-  /usr/libexec/PlistBuddy -c "Merge $RELEASE_ENTITLEMENTS" "$MERGED_ENTITLEMENTS" >/dev/null || true
+  merge_plist_allow_duplicates_only "$PROFILE_ENTITLEMENTS" "$MERGED_ENTITLEMENTS"
+  merge_plist_allow_duplicates_only "$RELEASE_ENTITLEMENTS" "$MERGED_ENTITLEMENTS"
🤖 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/upload-testflight.sh` around lines 540 - 547, Replace the blind
"|| true" around the two PlistBuddy Merge invocations so genuine failures still
abort: run /usr/libexec/PlistBuddy -c "Merge $PROFILE_ENTITLEMENTS"
"$MERGED_ENTITLEMENTS" and the same for $RELEASE_ENTITLEMENTS while capturing
stdout/stderr and the exit code; if exit is non-zero, inspect the captured
output and only suppress the error when it contains the specific "Duplicate
Entry Was Skipped" message, otherwise print the PlistBuddy output and exit
non‑zero so the script fails fast; update the merge logic surrounding
PROFILE_ENTITLEMENTS, RELEASE_ENTITLEMENTS and MERGED_ENTITLEMENTS accordingly.

-derivedDataPath "$out/DerivedData" \
PRODUCT_BUNDLE_IDENTIFIER="$BETA_BUNDLE_ID" \
CURRENT_PROJECT_VERSION="$build_number" \
${MARKETING_VERSION_OVERRIDE:+MARKETING_VERSION="$MARKETING_VERSION_OVERRIDE"} \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 The double-quote characters inside ${:+…} are not shell quoting delimiters — they are literal characters in the expansion result. When MARKETING_VERSION_OVERRIDE=1.0.2, bash produces the 20-character string MARKETING_VERSION="1.0.2" (quotes included) and passes it verbatim to xcodebuild. xcodebuild splits on = and assigns the value "1.0.2" (with quote characters) to MARKETING_VERSION, so the archive carries the wrong version string. Because the validation regex already ensures only digits and dots, the quotes have no safety purpose and should be removed.

Suggested change
${MARKETING_VERSION_OVERRIDE:+MARKETING_VERSION="$MARKETING_VERSION_OVERRIDE"} \
${MARKETING_VERSION_OVERRIDE:+MARKETING_VERSION=$MARKETING_VERSION_OVERRIDE} \

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
ios/scripts/cloud-testflight.sh (1)

119-123: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard --marketing-version before shift 2 to avoid premature parser exit.

Line 105 still shifts before confirming a value exists. With set -e, --marketing-version at argv end can terminate on shift before the Line 119-123 validation path runs, so users get a shell failure instead of your explicit error. Validate presence/non-flag first, then shift.

As per coding guidelines, "**/*.{ts,js,sh}: Production non-Swift app/runtime code must not use hacky sleeps..." (checked; not violated here) and this comment focuses on deterministic shell-argument correctness in the changed segment.

Suggested patch
-    --marketing-version) MARKETING_VERSION_OVERRIDE="${2:-}"; shift 2 ;;
+    --marketing-version)
+      [[ $# -ge 2 ]] || die "--marketing-version requires a value (X.Y or X.Y.Z)"
+      [[ "${2:-}" != -* ]] || die "--marketing-version requires a value (X.Y or X.Y.Z), got flag: ${2:-}"
+      MARKETING_VERSION_OVERRIDE="$2"
+      shift 2
+      ;;
🤖 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/cloud-testflight.sh` around lines 119 - 123, The script currently
performs shift (shift 2) for the --marketing-version option before confirming a
value exists, which can cause set -e to exit if the option is last; change the
parsing so you validate the next argument is present and not another flag before
shifting: when encountering the --marketing-version case, check that "$2" is
non-empty and does not start with '-' (or explicitly test that it matches the
expected version regex), assign it to MARKETING_VERSION_OVERRIDE, then run the
existing regex validation (MARKETING_VERSION_OVERRIDE =~
^[0-9]+(\.[0-9]+){1,2}$) and only after passing that validation perform shift 2;
reference the MARKETING_VERSION_OVERRIDE variable and the shift invocation to
locate and update the parsing branch.
🤖 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.

Duplicate comments:
In `@ios/scripts/cloud-testflight.sh`:
- Around line 119-123: The script currently performs shift (shift 2) for the
--marketing-version option before confirming a value exists, which can cause set
-e to exit if the option is last; change the parsing so you validate the next
argument is present and not another flag before shifting: when encountering the
--marketing-version case, check that "$2" is non-empty and does not start with
'-' (or explicitly test that it matches the expected version regex), assign it
to MARKETING_VERSION_OVERRIDE, then run the existing regex validation
(MARKETING_VERSION_OVERRIDE =~ ^[0-9]+(\.[0-9]+){1,2}$) and only after passing
that validation perform shift 2; reference the MARKETING_VERSION_OVERRIDE
variable and the shift invocation to locate and update the parsing branch.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ac555e62-80bd-48d0-a0a6-45e9b8bd8b03

📥 Commits

Reviewing files that changed from the base of the PR and between 594f2f1 and f6aad7f.

📒 Files selected for processing (1)
  • ios/scripts/cloud-testflight.sh

lawrencecchen and others added 3 commits June 11, 2026 18:43
TestFlight orders builds by marketing version before build number, so an
upload below the testers' installed marketing version is never offered as
an update. The 2026-06-10 cuts went out as 1.0.0 while testers ran 1.0.1
and were invisible. Plumb an explicit override through both the cloud
(BETA_MARKETING_VERSION env to reload-cloud-ios) and local archive paths.
… (ASC 90163)

The 2026-06-11 beta cut was rejected by App Store Connect with error 90163:
the re-sign merged the full Config/cmux-release.entitlements, which carries
com.apple.developer.usernotifications.time-sensitive, but the installed
"cmux Beta Distribution" profile predates that capability and does not
authorize the key. The same naive baseline+Release merge also silently
shipped without keychain-access-groups (authorized by the profile, present
in the accepted 2026-06-10 upload, but absent from both merge inputs).

Fix the merge in both directions: seed from the embedded profile's
Entitlements dict (the exact set ASC validates against), then merge the
Release file, then drop any key the profile does not authorize, warning per
dropped key. Replayed against the rejected export's artifacts this produces
a set byte-identical to the accepted 2026-06-10 upload. Restoring a dropped
capability (e.g. time-sensitive) requires regenerating the profile so it
snapshots the App ID's current capabilities; the capability itself is
already enabled on dev.cmux.app.beta.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…RSION env

Review follow-ups: reject malformed overrides (including a flag swallowed
by '--marketing-version --external') before the fleet build starts, and
explicitly unset BETA_MARKETING_VERSION when no override is requested so
an ambient value cannot leak into the cloud build.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lawrencecchen
lawrencecchen force-pushed the feat-beta-version-bump branch from f6aad7f to 7f93496 Compare June 12, 2026 01:43
@lawrencecchen
lawrencecchen merged commit aee3d36 into main Jun 12, 2026
24 of 25 checks passed

This branch was successfully deployed

1 active deployment
Preview – cmux — 7f93496e Deployed Jun 12, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant