-
Notifications
You must be signed in to change notification settings - Fork 180
allow staging pull requests into main #1263
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughRenames a branch gate workflow to enforce staging→main PRs, updates mobile deployment workflow to use timestamped branch names and an existing-PR check, and reworks the release calendar to create PRs directly from staging to main instead of using dedicated release branches. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub PR Check
Dev->>GH: Open PR to main
GH->>GH: Check head_ref == "staging"
alt head_ref != "staging"
GH-->>Dev: Fail check (blocked)
else head_ref == "staging"
GH-->>Dev: Pass check
end
sequenceDiagram
autonumber
participant WF as mobile-deploy.yml
participant GH as GitHub API (gh)
participant Repo as Git
WF->>WF: Compute VERSION, TIMESTAMP
WF->>Repo: Create branch ci/bump-mobile-version-${VERSION}-${TIMESTAMP}
WF->>GH: gh pr list --search "is:open ... ${VERSION}"
alt Existing PR found
GH-->>WF: Return PR URL
WF-->>WF: Output URL and exit
else No PR found
WF->>Repo: Commit version bump and push
WF->>GH: gh pr create (staging target)
GH-->>WF: PR URL
end
sequenceDiagram
autonumber
participant Scheduler as Release Calendar
participant GH as GitHub API (gh)
Scheduler->>GH: Check for open PR head=staging base=main
alt PR exists
GH-->>Scheduler: PR URL (no action)
else No PR
Scheduler->>GH: Create PR from staging to main
GH-->>Scheduler: PR URL
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/mobile-deploy.yml (1)
969-974: Use our composite cache action instead ofactions/cache.Per the workflow guidelines, we should rely on the shared composite actions under
.github/actionsfor dependency caching. Please replace this directactions/cache@v4usage with the appropriate in-repo composite (or add one for the NDK cache if it doesn’t exist yet) so we stay within policy.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/block-non-dev-to-main.yml(2 hunks).github/workflows/mobile-deploy.yml(1 hunks).github/workflows/release-calendar.yml(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.github/workflows/**/*.{yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
.github/workflows/**/*.{yml,yaml}: In GitHub workflows, use the shared composite actions in .github/actions for dependency caching instead of calling actions/cache directly
Use the cache-yarn composite action for Yarn dependency caching in workflows
Use the cache-bundler composite action for Ruby gems caching in workflows
Use the cache-gradle composite action for Gradle caching in workflows
Use the cache-pods composite action for CocoaPods caching in workflows
Files:
.github/workflows/mobile-deploy.yml.github/workflows/block-non-dev-to-main.yml.github/workflows/release-calendar.yml
🪛 actionlint (1.7.7)
.github/workflows/block-non-dev-to-main.yml
12-12: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions for more details
(expression)
| - name: Block PRs not from staging | ||
| run: | | ||
| if [[ "${{ github.head_ref }}" != "dev" ]]; then | ||
| echo "You can only merge from dev to main." | ||
| if [[ "${{ github.head_ref }}" != "staging" ]]; then | ||
| echo "You can only merge from staging to main." | ||
| exit 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Harden the head ref check to avoid command injection.
Interpolating ${{ github.head_ref }} directly inside the shell script leaves the step vulnerable to command injection if the branch name is crafted maliciously (see GitHub’s security guidance). Please pass the value through env: and reference the environment variable inside the script instead.
Apply this diff:
steps:
- - name: Block PRs not from staging
- run: |
- if [[ "${{ github.head_ref }}" != "staging" ]]; then
+ - name: Block PRs not from staging
+ env:
+ PR_HEAD_REF: ${{ github.head_ref }}
+ run: |
+ if [[ "${PR_HEAD_REF}" != "staging" ]]; then
echo "You can only merge from staging to main."
exit 1
fi🧰 Tools
🪛 actionlint (1.7.7)
12-12: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions for more details
(expression)
🤖 Prompt for AI Agents
.github/workflows/block-non-dev-to-main.yml lines 11-15: the workflow
interpolates ${{ github.head_ref }} directly into the shell which can allow
command injection; change the step to pass github.head_ref via env (e.g., env:
HEAD_REF: ${{ github.head_ref }}) and inside the run script reference the safe
environment variable with proper quoting and a plain string comparison (e.g., if
[[ "$HEAD_REF" != "staging" ]]; then ... exit 1; fi) to avoid executing any
injected content.
| EXISTING_PR=$(gh pr list --base "${TARGET_BRANCH}" --state open --json number,title,headRefName --jq ".[] | select(.title | contains(\"${VERSION}\")) | .number" | head -1) | ||
| if [ -n "$EXISTING_PR" ]; then | ||
| echo "⚠️ PR #${EXISTING_PR} already exists for version ${VERSION}" | ||
| echo "ℹ️ Skipping PR creation to avoid duplicates" | ||
| echo "ℹ️ Existing PR: https://github.com/${{ github.repository }}/pull/${EXISTING_PR}" | ||
| exit 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tighten the duplicate PR detection to avoid false skips.
Filtering on title | contains("${VERSION}") will skip creating the version bump PR whenever any other open PR to ${TARGET_BRANCH} mentions the same version in its title (e.g., a manually curated release doc), which blocks the automation. Please key off the head branch name pattern instead so we only skip genuine duplicates.
Apply this diff:
- EXISTING_PR=$(gh pr list --base "${TARGET_BRANCH}" --state open --json number,title,headRefName --jq ".[] | select(.title | contains(\"${VERSION}\")) | .number" | head -1)
+ EXISTING_PR=$(gh pr list --base "${TARGET_BRANCH}" --state open --json number,headRefName --jq ".[] | select(.headRefName | startswith(\"ci/bump-mobile-version-${VERSION}\")) | .number" | head -1)📝 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.
| EXISTING_PR=$(gh pr list --base "${TARGET_BRANCH}" --state open --json number,title,headRefName --jq ".[] | select(.title | contains(\"${VERSION}\")) | .number" | head -1) | |
| if [ -n "$EXISTING_PR" ]; then | |
| echo "⚠️ PR #${EXISTING_PR} already exists for version ${VERSION}" | |
| echo "ℹ️ Skipping PR creation to avoid duplicates" | |
| echo "ℹ️ Existing PR: https://github.com/${{ github.repository }}/pull/${EXISTING_PR}" | |
| exit 0 | |
| EXISTING_PR=$(gh pr list --base "${TARGET_BRANCH}" --state open --json number,headRefName --jq ".[] | select(.headRefName | startswith(\"ci/bump-mobile-version-${VERSION}\")) | .number" | head -1) | |
| if [ -n "$EXISTING_PR" ]; then | |
| echo "⚠️ PR #${EXISTING_PR} already exists for version ${VERSION}" | |
| echo "ℹ️ Skipping PR creation to avoid duplicates" | |
| echo "ℹ️ Existing PR: https://github.com/${{ github.repository }}/pull/${EXISTING_PR}" | |
| exit 0 |
🤖 Prompt for AI Agents
In .github/workflows/mobile-deploy.yml around lines 1332-1338, the script
currently detects existing PRs by checking if the PR title contains the VERSION
which leads to false positives; update the gh pr list jq filter to inspect
headRefName instead (e.g., select(.headRefName | contains("${VERSION}")) or
match a specific branch-name pattern your workflow uses) so the check only skips
when an open PR was created from a branch that actually corresponds to this
version; replace the title-based select with a headRefName-based select and keep
the rest of the logic (echo and exit) unchanged.
Summary by CodeRabbit