Repository navigation
Limit CMUX INTERNAL builds to iOS changes - #8812
Conversation
📝 WalkthroughWalkthroughThe TestFlight workflow now runs on ChangesiOS TestFlight path gating
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 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 |
![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 `@tests/test_ios_testflight_main_push_filter.py`:
- Around line 32-50: Update test_main_push_triggers_only_for_ios_affecting_paths
and its trigger parsing helpers to isolate the push configuration structurally,
including both push.branches and push.paths. Parse quoted and unquoted YAML
entries consistently, then assert branches equal exactly ("main",) and paths
equal exactly IOS_PATHS instead of relying on partial string matches.
🪄 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 Plus
Run ID: 65440564-d5a6-425e-9acc-2ba7d36f8660
📒 Files selected for processing (4)
.github/workflows/ci.yml.github/workflows/ios-testflight.ymlios/scripts/generate-testflight-notes.shtests/test_ios_testflight_main_push_filter.py
Greptile SummaryThis PR narrows the automatic CMUX INTERNAL TestFlight trigger from every main push to only pushes that touch iOS-relevant files (sources, shared packages, GhosttyKit inputs, related build scripts, and the workflow itself). Manual
Confidence Score: 5/5Safe to merge — the change is limited to CI trigger logic and test scaffolding with no production Swift or runtime code touched. All five changed files are GitHub Actions workflow YAML, a bash notes-generation script, and Python tests. The path filter is correctly mirrored between the workflow and the shell script, the new test enforces that contract in CI, and the now-contradictory every-push test is cleanly removed. No correctness, security, or runtime concerns were found. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Push to main branch] --> B{Paths filter match?}
B -->|"iOS source / scripts / workflow touched"| C[Queue ios-testflight workflow\nConcurrency key: SHA]
B -->|"Non-iOS change only"| D[Workflow skipped — no upload]
E[workflow_dispatch] --> F[Queue ios-testflight workflow\nConcurrency key: run_id]
C --> G{shouldBuild?}
F --> G
G -->|"push or workflow_dispatch"| H[Build & upload CMUX INTERNAL\nto TestFlight]
H --> I[generate-testflight-notes.sh\nuses same PATHS list\nfor 'What to Test' notes]
Reviews (3): Last reviewed commit: "Validate TestFlight trigger keys exactly" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def trigger_paths(triggers: str) -> tuple[str, ...]: | ||
| lines = triggers[triggers.index(" paths:\n") :].splitlines()[1:] | ||
| return tuple( | ||
| line.removeprefix(' - "').removesuffix('"') | ||
| for line in lines | ||
| if line.startswith(' - "') | ||
| ) |
There was a problem hiding this comment.
trigger_paths has no stop condition past the paths: list
The slice starts at paths:\n and runs to the end of the trigger block (bounded by concurrency:), meaning the workflow_dispatch input section is also scanned. Today none of those lines start with - ", so the filter is safe. However, if a future workflow_dispatch input description were wrapped in a YAML block sequence at 6-space indentation (e.g. - "some description"), it would be silently counted as a path entry and the assertion would either pass incorrectly or give a confusing mismatch. An explicit stop at the first non- - line after paths: would make the boundary deterministic.
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!
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.
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 `@tests/test_ios_testflight_main_push_filter.py`:
- Around line 70-72: Update the trigger assertions in the test to parse direct
keys under the YAML on: block structurally, using indentation-aware logic
consistent with mapping_block and defining mapping_keys. Assert that schedule is
absent from those parsed keys while preserving the workflow_dispatch check.
🪄 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 Plus
Run ID: 4d1336d7-a76b-4d6e-ab95-093878615cf3
📒 Files selected for processing (1)
tests/test_ios_testflight_main_push_filter.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. |
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 `@tests/test_ios_testflight_main_push_filter.py`:
- Around line 83-85: Update the assertions in the workflow trigger test around
mapping_keys, sequence_values, and IOS_PATHS to compare sorted collections
rather than their source order. Preserve exact membership and duplicate
detection while allowing workflow triggers and paths to be reordered.
🪄 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 Plus
Run ID: f24f860a-75d5-45d0-8678-e72f15f35c0b
📒 Files selected for processing (1)
tests/test_ios_testflight_main_push_filter.py
1 participant
Summary:
Tests:
python3 tests/test_ios_testflight_main_push_filter.pypython3 tests/test_ios_testflight_notes.py.github/workflows/ios-testflight.ymland.github/workflows/ci.ymlwith Ruby YAMLgit diff --checkNeed help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Limit automatic CMUX INTERNAL TestFlight uploads to iOS-affecting changes only. Manual dispatch remains, and TestFlight notes mirror the same path contract with CI checks.
pathsfilter in.github/workflows/ios-testflight.ymlforios/**,Packages/iOS/**,Packages/Shared/**,Sources/Mobile/**, GhosttyKit inputs (ghostty,ghostty.h), related scripts, and the workflow file.ios/scripts/generate-testflight-notes.sh; added tests to verify exact trigger keys and that workflow and notes paths match; enforced via a new CI step.run_id.Written for commit 7c09399. Summary will update on new commits.
Summary by CodeRabbit