Skip to content

ci: add permission control for public ci tests#2397

Merged
yzh119 merged 9 commits intoflashinfer-ai:mainfrom
yongwww:ci_permission
Jan 25, 2026
Merged

ci: add permission control for public ci tests#2397
yzh119 merged 9 commits intoflashinfer-ai:mainfrom
yongwww:ci_permission

Conversation

@yongwww
Copy link
Member

@yongwww yongwww commented Jan 22, 2026

📌 Description

#2355

Add team-based permission control for the PR test workflow to manage CI costs.

  • Add gate job to check authorization before running tests
  • PRs from ci-users team members run automatically
  • External PRs require run-ci label (via @flashinfer-bot run command from member of ci-users)
  • Add ci-bot-commands.yml for handling bot commands:
    • @flashinfer-bot run - trigger CI
    • @flashinfer-bot rerun - rerun all jobs
    • @flashinfer-bot rerun failed - rerun failed and cancelled jobs
  • Add pr-label-cleanup.yml to auto-remove run-ci label when external PRs receive new commits

🔍 Related Issues

🚀 Pull Request Checklist

Thank you for contributing to FlashInfer! Before we review your pull request, please make sure the following items are complete.

✅ Pre-commit Checks

  • I have installed pre-commit by running pip install pre-commit (or used your preferred method).
  • I have installed the hooks with pre-commit install.
  • I have run the hooks manually with pre-commit run --all-files and fixed any reported issues.

If you are unsure about how to set up pre-commit, see the pre-commit documentation.

🧪 Tests

  • Tests have been added or updated as needed.
  • All tests are passing (unittest, etc.).

Co-Authored-By: Kevin Vincent kvincent@nvidia.com

Reviewer Notes

Summary by CodeRabbit

  • New Features

    • Add PR comment bot commands to trigger, rerun (all or failed), or stop CI, with reaction feedback.
  • Chores

    • Introduce CI authorization gating so CI only runs when authorized.
    • Expand PR workflow triggers and add skip messaging when CI is gated.
  • Bug Fixes / Maintenance

    • Automatically remove CI labels from external contributors and post explanatory guidance.

✏️ Tip: You can customize this high-level summary in your review settings.

@gemini-code-assist
Copy link
Contributor

Note

Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 22, 2026

📝 Walkthrough

Walkthrough

Adds three GitHub Actions workflows: a bot-driven PR command processor triggered by issue comments that authorizes ci-users to control CI, a PR label cleanup workflow that removes run-ci for external authors, and a gated PR test workflow that checks authorization before running CI jobs.

Changes

Cohort / File(s) Summary
CI Automation Bot Commands
.github/workflows/ci-bot-commands.yml
New workflow handling issue_comment events mentioning the bot; verifies ci-users membership via token, parses commands (run, rerun, rerun-failed, stop), and conditionally applies run-ci, reruns workflows, cancels runs, or reacts when unauthorized; includes error handling and reactions.
PR Label Cleanup
.github/workflows/pr-label-cleanup.yml
New workflow on pull_request synchronize that checks author_association, and removes the run-ci label and comments when the author is external; writes is_external output.
PR Test Workflow Enhancement
.github/workflows/pr-test.yml
Added gate job exporting authorized (team membership / label checks), extended PR triggers, added pull-requests: write permission, wired gating into setup, aot-build-import, gpu-tests-* and test-results-summary via needs and if, and posts skip messaging when unauthorized.

Sequence Diagram(s)

sequenceDiagram
    participant User as PR Commenter
    participant GitHub as GitHub API
    participant Workflow as CI-Bot Workflow
    participant TeamAPI as GitHub Team API
    participant CI as CI System

    User->>GitHub: Post comment mentioning bot
    GitHub->>Workflow: Trigger issue_comment event
    Workflow->>Workflow: Verify PR context & bot mention
    Workflow->>TeamAPI: Check ci-users membership
    TeamAPI-->>Workflow: Member / Not member

    alt Authorized
        Workflow->>Workflow: Parse command (run / rerun / rerun-failed / stop)
        alt run
            Workflow->>GitHub: Add `run-ci` label
        else rerun
            Workflow->>CI: Rerun all workflows for PR commit
        else rerun-failed
            Workflow->>CI: Rerun failed workflows for PR commit
        else stop
            Workflow->>CI: Cancel in-progress/queued runs for PR commit
        end
        Workflow->>GitHub: Post thumbs-up reaction
    else Unauthorized
        Workflow->>GitHub: Post confused reaction
    end
Loading
sequenceDiagram
    participant GitHubEvents as GitHub Events
    participant Gate as Gate Job
    participant Setup as Setup Job
    participant CIJobs as CI Jobs
    participant Results as Test Results

    GitHubEvents->>Gate: PR opened/synchronize/reopened/labeled or workflow_dispatch
    Gate->>Gate: Check `run-ci` label and ci-users membership
    Gate-->>Setup: Export `authorized` flag

    alt Authorized
        Gate->>Setup: Allow setup to run
        Setup->>CIJobs: Trigger gated CI jobs (aot-build-import, gpu-tests-*)
        CIJobs->>Results: Produce test results
    else Unauthorized
        Gate->>GitHubEvents: Post skip message on PR
        Setup--xCIJobs: Skip (blocked by gate)
        Results->>GitHubEvents: Indicate CI skipped
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Suggested reviewers

  • yzh119
  • bkryu
  • jimmyzho

Poem

🐰 I hop through commits, label in paw,
I listen for commands that open the maw,
"Run" and "Rerun" I check who may call,
Gate swings for trusted friends, otherwise stall,
Thumbs-up and carrots — CI hops to the ball.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding permission control for public CI tests, which aligns with the PR objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The PR description addresses the template requirements with a clear description, issue reference, and completed pre-commit checklist; however, the Tests section is incomplete with only one item checked.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @.github/workflows/ci-bot-commands.yml:
- Around line 64-77: The Parse command step currently interpolates the untrusted
comment body directly into the shell (variable BODY="${{
github.event.comment.body }}"), enabling command injection; instead, stop inline
YAML interpolation and load the comment body from a safe environment or file
(e.g., set an env var or read/parsing from GITHUB_EVENT_PATH) and then perform
the grep checks against that safe variable; update the step named "Parse
command" (id: parse) to populate BODY from an environment variable or
JSON-parsed file before running the echo/grep comparisons so no untrusted
content is directly injected into the shell.
🧹 Nitpick comments (2)
.github/workflows/pr-test.yml (1)

117-120: Duplicate comment check may be fragile.

The grep -c "CI is paused" check could produce false positives if someone quotes this phrase in another comment. Consider using a more unique marker (e.g., an HTML comment like <!-- ci-skip-notice -->).

Suggested improvement
-            --jq '.comments[].body' | grep -c "CI is paused" || true)
+            --jq '.comments[].body' | grep -c "<!-- ci-gate-notice -->" || true)
...
-              --body "**CI is paused for external contributors.**
+              --body "<!-- ci-gate-notice -->
+**CI is paused for external contributors.**
.github/workflows/ci-bot-commands.yml (1)

148-160: RERUN_COUNT won't reflect actual count due to subshell.

The while read loop runs in a subshell (due to the pipe), so RERUN_COUNT modifications are lost. This is cosmetic since the counter isn't used afterward, but it's worth noting for future debugging.

Fix using process substitution (if count is needed later)
          # Rerun failed workflow runs for this commit
          RERUN_COUNT=0
-         gh run list \
+         while read -r run_id; do
+           if [ -n "$run_id" ]; then
+             echo "Rerunning failed jobs in workflow $run_id..."
+             gh run rerun "$run_id" --repo ${{ github.repository }} --failed || true
+             RERUN_COUNT=$((RERUN_COUNT + 1))
+           fi
+         done < <(gh run list \
            --repo ${{ github.repository }} \
            --commit "$PR_SHA" \
            --status failure \
-           --json databaseId -q '.[].databaseId' | \
-         while read -r run_id; do
-           if [ -n "$run_id" ]; then
-             echo "Rerunning failed jobs in workflow $run_id..."
-             gh run rerun "$run_id" --repo ${{ github.repository }} --failed || true
-             RERUN_COUNT=$((RERUN_COUNT + 1))
-           fi
-         done
+           --json databaseId -q '.[].databaseId')

@yzh119
Copy link
Collaborator

yzh119 commented Jan 22, 2026

@yongwww can we test the functionality of the CI before it's merged?

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @.github/workflows/ci-bot-commands.yml:
- Around line 113-123: The current gh run list pipeline filters out only
"in_progress" runs and can pass non-completed statuses (like "queued") to gh run
rerun; update the jq filter used with gh run list (the command around gh run
list --commit "$PR_SHA" --json databaseId,status -q '.[] | select(.status !=
"in_progress") | .databaseId') to explicitly select only completed runs (e.g.,
change to select(.status == "completed") or an equivalent check for
terminal/completed statuses) so gh run rerun is invoked only for completed runs;
keep the surrounding while read loop and gh run rerun invocation unchanged
except for this jq filter adjustment.
🧹 Nitpick comments (1)
.github/workflows/ci-bot-commands.yml (1)

27-62: Use the team membership endpoint to avoid listing all members.
Fetching all members can be slow and API-heavy for large teams. The membership endpoint is cheaper and avoids pagination.

♻️ Proposed refactor
-          # List team members and check if commenter is in the list
-          MEMBERS=$(gh api \
-            -H "Accept: application/vnd.github+json" \
-            -H "X-GitHub-Api-Version: 2022-11-28" \
-            --paginate \
-            "/orgs/${ORG}/teams/${TEAM}/members" \
-            --jq '.[].login' 2>&1) || {
-            echo "::error::Failed to get team members: $MEMBERS"
-            echo "authorized=false" >> "$GITHUB_OUTPUT"
-            exit 0
-          }
-
-          if echo "$MEMBERS" | grep -qx "$ACTOR"; then
+          # Check membership directly
+          if gh api \
+            -H "Accept: application/vnd.github+json" \
+            -H "X-GitHub-Api-Version: 2022-11-28" \
+            "/orgs/${ORG}/teams/${TEAM}/memberships/${ACTOR}" >/dev/null 2>&1; then
             echo "$ACTOR is a member of $TEAM"
             echo "authorized=true" >> "$GITHUB_OUTPUT"
           else
-            echo "$ACTOR is not a member of $TEAM"
+            echo "$ACTOR is not a member of $TEAM or membership check failed"
             echo "authorized=false" >> "$GITHUB_OUTPUT"
           fi

@yongwww yongwww marked this pull request as draft January 23, 2026 18:36
@yongwww yongwww marked this pull request as ready for review January 24, 2026 00:38
@yongwww
Copy link
Member Author

yongwww commented Jan 24, 2026

@yongwww can we test the functionality of the CI before it's merged?

The issue_comment triggered workflow reads from the default branch (main), not the PR branch, so I can't test the bot commands directly on this PR until it's merged. (Reference: This event will only trigger a workflow run if the workflow file exists on the default branch.)
However, I've tested the functionality in my fork (merged in main, and test in a pr): Test PR: yongwww-org#6.

@yzh119 yzh119 merged commit c8d76d3 into flashinfer-ai:main Jan 25, 2026
65 of 74 checks passed
@yongwww yongwww deleted the ci_permission branch January 25, 2026 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants