Repository navigation
Upload cmux INTERNAL for every main push - #8694
Conversation
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
📝 WalkthroughWalkthroughThe iOS TestFlight workflow now uploads builds for every ChangesEvery-main-push TestFlight lane
Settings property ordering
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant decide
participant upload
participant TestFlight
GitHubActions->>decide: Start on main push or manual dispatch
decide->>GitHubActions: Wait for earlier main uploads
decide->>upload: Enable build with notes base SHA
upload->>TestFlight: Upload CMUX internal build
GitHubActions->>TestFlight: Assign uploaded build to internal group
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 25✅ Passed checks (25 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 |
Greptile SummaryThis PR shifts the iOS TestFlight lane from a scheduled (every-2h) batch model to a per-push model: every commit to
Confidence Score: 5/5Safe to merge — the workflow logic is internally consistent, the ordering mechanism correctly serializes uploads by polling job status rather than relying on wall-clock guesses, and the Swift fix addresses a real compile failure without introducing any new debug seams or ambient global state. The Swift change is a one-line relocation that resolves a confirmed Release-mode compile error (the property had a production caller outside its #if DEBUG guard). The workflow rewrite is well-documented, the ordering loop correctly keys on run ID comparison and job-status polling rather than wall-clock assumptions, and the tests pin the critical invariants. No correctness bugs were found. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant GH as GitHub (push to main)
participant D as decide job(Linux, up to 6h)
participant API as GitHub Actions API
participant U as upload job(macOS)
participant ASC as App Store Connect
participant A as assign-internal-group job
GH->>D: trigger (per-SHA concurrency group)
loop every 60s until no earlier upload is blocking
D->>API: listWorkflowRuns(branch:main, per_page:100)
API-->>D: earlier active runs
D->>API: listJobsForWorkflowRun(run_id)
API-->>D: job status for Upload to TestFlight
end
D->>D: resolve last_uploaded_sha (notes base)
D-->>U: "should_build=true, last_uploaded_sha"
U->>U: xcodebuild archive + export
U->>ASC: upload IPA (monotonic timestamp build number)
ASC-->>U: build accepted
U-->>A: final_build_number
A->>ASC: assign build to internal TestFlight group
Reviews (3): Last reviewed commit: "Remove stale TestFlight schedule event" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| run.status !== 'completed' && | ||
| ['push', 'workflow_dispatch', 'schedule'].includes(run.event) |
There was a problem hiding this comment.
'schedule' event in this filter is a leftover from the removed cron trigger. Since the workflow no longer has a schedule: trigger, no run will ever have run.event === 'schedule', so the entry is dead code. Removing it keeps the filter aligned with the actual trigger surface.
| run.status !== 'completed' && | |
| ['push', 'workflow_dispatch', 'schedule'].includes(run.event) | |
| run.status !== 'completed' && | |
| ['push', 'workflow_dispatch'].includes(run.event) |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ios-testflight.yml (1)
113-123: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winStale downstream comment/dead artifact once assign-only retry logic is removed here.
This range confirms the decide job's assignment-retry variables/outputs are removed. Further down in this same file, the
assign-internal-groupjob still has a comment ("the decide job's assign-only retry logic looks for this exact artifact name") and an "Upload assignment-state artifact" step producingios-testflight-assignment-state-complete— but nothing in the redesigneddecidejob reads that artifact anymore, and the492-496change confirmsBUILD_NUMBERnow flows directly fromneeds.upload.outputs.final_build_numberrather than an artifact restore path. That comment and artifact upload appear vestigial now and should be updated or removed to avoid misleading future maintainers.🤖 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-testflight.yml around lines 113 - 123, Remove the obsolete assignment-state artifact upload from the assign-internal-group job, along with the comment claiming the decide job consumes it. Keep BUILD_NUMBER sourced directly from needs.upload.outputs.final_build_number and remove only the now-unused artifact-related logic.
🤖 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-testflight.yml:
- Around line 42-100: Wrap the earlier-run polling logic in the decide step
around the workflow and job API calls with transient-error handling, following
the graceful behavior used by the “resolve last uploaded sha” loop. On API
failure, log the error and continue waiting/retrying rather than allowing the
exception to fail the job; preserve the existing timeout failure and ensure
control reaches should_build output for a successful decision.
---
Outside diff comments:
In @.github/workflows/ios-testflight.yml:
- Around line 113-123: Remove the obsolete assignment-state artifact upload from
the assign-internal-group job, along with the comment claiming the decide job
consumes it. Keep BUILD_NUMBER sourced directly from
needs.upload.outputs.final_build_number and remove only the now-unused
artifact-related logic.
🪄 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: 12f6cf66-f341-4f04-9835-0533f4f651d3
📒 Files selected for processing (4)
.github/workflows/ios-testflight.ymlPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileSettingsView.swiftios/scripts/upload-testflight.shtests/test_ios_testflight_every_main_push.py
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/ios-testflight.yml (2)
60-66: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPaginate the workflow-run query before proceeding.
listWorkflowRuns(..., per_page: 100)inspects only the first page. With more than 100 queued/newer runs, an older active upload can be omitted, allowing concurrent App Store Connect uploads and violating monotonic build-number ordering. Continue paging until all earlier runs are covered, or use an authoritative upload queue.As per coding guidelines, identify scalable collections and avoid incomplete full scans.
🤖 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-testflight.yml around lines 60 - 66, Update the workflow-run lookup using github.rest.actions.listWorkflowRuns so it paginates through all pages rather than inspecting only per_page: 100 results. Use the complete run collection when determining earlier active uploads, preserving the existing filtering and ordering behavior before proceeding.Source: Coding guidelines
57-86: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftAvoid the per-run API fan-out on every polling pass.
For
Rqueued runs, each 60-second iteration performs roughly1 + RAPI calls, and acrossRwaiting workflow runs this becomes O(R²) calls. Merge bursts can exhaust GitHub API limits and cause the entire upload lane to fail. Prefer a durable predecessor/queue mechanism; otherwise bound the cohort and avoid querying each run’s jobs independently.As per coding guidelines, avoid repeated full scans and per-item nested scans over scalable collections.
🤖 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-testflight.yml around lines 57 - 86, Replace the per-poll `listJobsForWorkflowRun` fan-out inside the `earlierActiveRuns` loop with a durable predecessor or queue-based mechanism that identifies the immediate blocking run without scanning every predecessor’s jobs. If that mechanism is unavailable, bound the candidate cohort and reuse a single bounded workflow/job query rather than issuing one jobs request per run on every polling iteration; preserve the existing ordering and blocking behavior.Source: Coding guidelines
🤖 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.
Outside diff comments:
In @.github/workflows/ios-testflight.yml:
- Around line 60-66: Update the workflow-run lookup using
github.rest.actions.listWorkflowRuns so it paginates through all pages rather
than inspecting only per_page: 100 results. Use the complete run collection when
determining earlier active uploads, preserving the existing filtering and
ordering behavior before proceeding.
- Around line 57-86: Replace the per-poll `listJobsForWorkflowRun` fan-out
inside the `earlierActiveRuns` loop with a durable predecessor or queue-based
mechanism that identifies the immediate blocking run without scanning every
predecessor’s jobs. If that mechanism is unavailable, bound the candidate cohort
and reuse a single bounded workflow/job query rather than issuing one jobs
request per run on every polling iteration; preserve the existing ordering and
blocking behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1311135b-deaf-409c-b596-867b711a417e
📒 Files selected for processing (2)
.github/workflows/ios-testflight.ymltests/test_ios_testflight_every_main_push.py
1 participant
What changed
main, without path or SHA skip gates.ToastCenteravailable outside DEBUG builds.This deliberately replaces the batching policy from #6214 because the required contract is now one internal build per main change.
Verification
python3 tests/test_ios_testflight_every_main_push.pygo run github.com/rhysd/actionlint/cmd/actionlint@v1.7.7 .github/workflows/ios-testflight.ymltfmain: passedcmux-tfmain-codexsimulator: passedNeed help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Ships a cmux INTERNAL TestFlight build for every push to main, and queues uploads so build numbers stay monotonic. Also removes the old schedule lane and fixes a Release archive failure.
New Features
run_id.workflow_dispatchstays, with optional build number and marketing version overrides.dev.cmux.app.internal, display name “cmux INTERNAL”, and internal group assignment.Bug Fixes
ToastCenteravailable in Release builds to fix the Release archive failure.Written for commit 6dc0ad1. Summary will update on new commits.
Summary by CodeRabbit
New Features
main(manual builds still available).Tests
Documentation
Refactor