Repository navigation
ci: gate expensive jobs on 'ci-approved' label for fork PRs - #1232
CatherineSue wants to merge 1 commit into
Conversation
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
📝 WalkthroughWalkthroughUpdated GitHub Actions workflow configuration in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Hi @CatherineSue, this PR has merge conflicts that must be resolved before it can be merged. Please rebase your branch: git fetch origin main
git rebase origin/main
# resolve any conflicts, then:
git push --force-with-lease |
mergify
Bot
added
the
needs-rebase
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c188c84193
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| github.event_name != 'pull_request' || | ||
| github.event.action != 'labeled' || | ||
| github.event.label.name == 'ci-approved' |
There was a problem hiding this comment.
This gate now only passes on labeled events when the newly added label is exactly ci-approved, so a PR that is already approved can still have the entire workflow skipped if any other label is added. With the workflow’s concurrency.cancel-in-progress: true, that skipped run can cancel an in-flight synchronize run and leave the latest commit with only skipped checks and no replacement test run. Previously the gate used contains(..., 'ci-approved'), so once approved, additional labels did not suppress CI.
Useful? React with 👍 / 👎.
Adds an if: condition on build-wheel that skips expensive GPU and paid-API jobs for fork PRs unless they carry the 'ci-approved' label. Internal PRs run unconditionally. build-wheel is the first-tier expensive job; benchmarks, e2e-* (1/2/4-GPU), e2e-vendor (paid Anthropic/OpenAI/xAI APIs), python-unit-tests, go-unit-tests, and go-bindings-e2e all inherit the gate transitively through their needs: build-wheel chain. Does not reintroduce pull_request_target or the prior ci-gate job (both reverted in #1175). To kick off expensive jobs on a fork PR after adding the label: wait for the contributor's next push (synchronize event), or click "Re-run all jobs" in the Actions UI. Recommended companion setting (repo admin): Settings -> Actions -> General -> Fork pull request workflows from outside collaborators: change to "Require approval for first-time contributors" so returning fork contributors don't need the manual approve-and-run click on every push. Signed-off-by: Chang Su <chang.s.su@oracle.com>
c188c84 to
1789cd0
Compare
CatherineSue
changed the title
mergify
Bot
removed
the
needs-rebase
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1789cd065b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-test-rust.yml:
- Around line 106-113: The build-wheel job is missing cheap-tier dependencies,
allowing expensive work to run before quick prechecks finish; update the
workflow so the build-wheel job (job id "build-wheel") includes a needs: array
referencing the cheap precheck jobs (e.g., "pre-commit", "python-lint",
"grpc-proto-build-check", "unit-tests") so it only runs after those succeed,
preserving the intended fail-fast/cost gate.
- Around line 106-113: The workflow's fork-PR gate (the if: condition using
contains(github.event.pull_request.labels.*.name, 'ci-approved')) won't
self-unblock because the workflow doesn't listen for pull_request labeled
events; update the workflow's triggers so pull_request includes types: [opened,
synchronize, reopened, labeled] (or add labeled to the existing types list) so
that adding the 'ci-approved' label will re-run the workflow and allow the
contains(...) check to evaluate; ensure the change is made where pull_request
types are declared in the YAML.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: af523fba-5050-4783-851d-72e02a6c8072
📒 Files selected for processing (1)
.github/workflows/pr-test-rust.yml
1 participant
Description
Problem
After #1175 reverted
pull_request_targetand removed theci-approvedgate entirely, fork PRs that pass the one-time "Approve and run" check now run the full expensive tier on every push — GPU runners (build-wheel,benchmarks, alle2e-*,go-bindings-e2e) and paid third-party APIs (e2e-vendorhits Anthropic, OpenAI, xAI). That's uncapped cost exposure on approved-but-not-yet-reviewed fork contributions.Solution
Gate the first-tier expensive job (
build-wheel) on aci-approvedlabel for fork PRs. Everything downstream inherits the gate vianeeds: build-wheel. Internal (non-fork) PRs run unconditionally — no behavior change for maintainers.Minimal, additive change only. Does not reintroduce
pull_request_target, the priorci-gatejob, or anylabeled-type trigger.Changes
pr-test-rust.yml— singleif:onbuild-wheel:Behavior matrix:
ci-approvedlabelTrigger timing for fork PRs
Adding the
ci-approvedlabel alone does not re-trigger CI (this PR intentionally does not addlabeledtotypes:, to avoid skipped-run noise on unrelated labels). To kick off expensive jobs after labeling:synchronizeevent fires,if:picks up the label), orRecommended companion setting (repo admin)
Settings → Actions → General → Fork pull request workflows from outside collaborators: change from "Require approval for all outside collaborators" to "Require approval for first-time contributors". Without this, returning fork contributors still need a manual "Approve and run" click on every push.
Test Plan
if:short-circuits on!fork == true).ci-approved, verifybuild-wheeland everything downstream show asskipped.ci-approvedto that fork PR and verify (after a new push or manual Re-run) the full tier runs.Checklist
cargo +nightly fmtpasses (N/A — YAML-only change)cargo clippy --all-targets --all-features -- -D warningspasses (N/A — YAML-only change)Summary by CodeRabbit