From 36c902dc0d277e96d348b3c2d137ecd33767f341 Mon Sep 17 00:00:00 2001 From: sayakpaul Date: Thu, 4 Jun 2026 18:56:59 +0530 Subject: [PATCH 1/3] start --- .github/workflows/pr_comment_gpu_tests.yml | 199 +++++++++++++++++ .github/workflows/run_tests_from_a_pr.yml | 76 ------- diffusers-bot-gpu-tests-plan.md | 241 +++++++++++++++++++++ 3 files changed, 440 insertions(+), 76 deletions(-) create mode 100644 .github/workflows/pr_comment_gpu_tests.yml delete mode 100644 .github/workflows/run_tests_from_a_pr.yml create mode 100644 diffusers-bot-gpu-tests-plan.md diff --git a/.github/workflows/pr_comment_gpu_tests.yml b/.github/workflows/pr_comment_gpu_tests.yml new file mode 100644 index 000000000000..d3f159653cae --- /dev/null +++ b/.github/workflows/pr_comment_gpu_tests.yml @@ -0,0 +1,199 @@ +name: GPU Tests from PR Comment + +# Lets maintainers (admin / write access) run GPU tests on a PR by commenting: +# /diffusers-bot pytest +# e.g. `/diffusers-bot pytest tests/models/test_modeling_common.py -k "some_test"`. + + +on: + issue_comment: + types: [created] + +# Default to read-only; jobs that comment opt into `pull-requests: write` explicitly. +permissions: + contents: read + +concurrency: + # A newer command on the same PR supersedes an in-flight one. + group: diffusers-bot-${{ github.event.issue.number }} + cancel-in-progress: true + +env: + DIFFUSERS_IS_CI: yes + OMP_NUM_THREADS: 8 + MKL_NUM_THREADS: 8 + HF_XET_HIGH_PERFORMANCE: 1 + PYTEST_TIMEOUT: 600 + # Force version overrides across every `uv pip install`: pin tokenizers and the + # torch/torchvision/torchaudio set baked into the image so `-U` installs can't bump + # torch and break torchvision's C++ ABI. Re-written into the file in the install step. + UV_OVERRIDE: /tmp/uv-overrides.txt + +jobs: + gate: + name: Authorize & launch + # Only react to `/diffusers-bot pytest …` comments on open PRs. + if: | + github.event.issue.pull_request && + github.event.issue.state == 'open' && + startsWith(github.event.comment.body, '/diffusers-bot pytest') + runs-on: ubuntu-22.04 + permissions: + pull-requests: write + outputs: + pytest_args: ${{ steps.parse.outputs.pytest_args }} + comment_id: ${{ steps.comment.outputs.comment_id }} + steps: + - name: Check commenter permission + id: auth + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + COMMENTER: ${{ github.event.comment.user.login }} + run: | + PERM=$(gh api "repos/${REPO}/collaborators/${COMMENTER}/permission" --jq '.permission' 2>/dev/null || echo "none") + echo "Commenter @${COMMENTER} has permission: ${PERM}" + if [[ "$PERM" == "admin" || "$PERM" == "write" ]]; then + echo "authorized=true" >> "$GITHUB_OUTPUT" + else + echo "authorized=false" >> "$GITHUB_OUTPUT" + fi + + - name: Reject unauthorized commenter + if: steps.auth.outputs.authorized != 'true' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + PR: ${{ github.event.issue.number }} + COMMENTER: ${{ github.event.comment.user.login }} + run: | + gh api -X POST "repos/${REPO}/issues/${PR}/comments" \ + -f body="🚫 Sorry @${COMMENTER}, you're not authorized to run \`/diffusers-bot\`. Only maintainers with write or admin access can trigger GPU tests." >/dev/null + echo "::error::Only maintainers with write/admin access can run /diffusers-bot." + exit 1 + + - name: Acknowledge with πŸ‘€ + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + COMMENT_ID: ${{ github.event.comment.id }} + run: | + gh api -X POST "repos/${REPO}/issues/comments/${COMMENT_ID}/reactions" -f content="eyes" >/dev/null + + - name: Parse pytest args + id: parse + env: + COMMENT_BODY: ${{ github.event.comment.body }} + run: | + # Use only the first line of the comment, strip the command prefix. + FIRST_LINE=$(printf '%s' "$COMMENT_BODY" | head -n1) + ARGS="${FIRST_LINE#/diffusers-bot pytest}" + # Trim surrounding whitespace/CR. + ARGS="$(printf '%s' "$ARGS" | sed 's/^[[:space:]]*//;s/[[:space:]]*$//')" + echo "pytest_args=${ARGS}" >> "$GITHUB_OUTPUT" + + - name: Post "running" comment + id: comment + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + PR: ${{ github.event.issue.number }} + COMMENTER: ${{ github.event.comment.user.login }} + PYTEST_ARGS: ${{ steps.parse.outputs.pytest_args }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + BODY="⏳ Running \`pytest ${PYTEST_ARGS}\` on a GPU runner β€” [view logs](${RUN_URL}). + + Triggered by @${COMMENTER}." + CID=$(gh api -X POST "repos/${REPO}/issues/${PR}/comments" -f body="$BODY" --jq '.id') + echo "comment_id=${CID}" >> "$GITHUB_OUTPUT" + + gpu_tests: + name: Run pytest on GPU + needs: gate + runs-on: + group: aws-g4dn-2xlarge + container: + image: diffusers/diffusers-pytorch-cuda + options: --gpus all --shm-size "16gb" --ipc host + # Least privilege: this job checks out and runs untrusted fork code, so it gets no + # write token. Comment writes happen only in `gate`/`report`. + permissions: + contents: read + defaults: + run: + shell: bash + steps: + - name: Checkout PR head + uses: actions/checkout@v6 + with: + # Works for forks too β€” no fork credentials needed. + ref: refs/pull/${{ github.event.issue.number }}/head + fetch-depth: 2 + + - name: NVIDIA-SMI + run: nvidia-smi + + - name: Install dependencies + run: | + printf 'tokenizers<0.23.0\ntorch==2.10.0\ntorchvision==0.25.0\ntorchaudio==2.10.0\n' > "$UV_OVERRIDE" + uv pip install -e ".[quality,training,test]" + uv pip install peft@git+https://github.com/huggingface/peft.git + uv pip uninstall accelerate && uv pip install -U accelerate@git+https://github.com/huggingface/accelerate.git + uv pip uninstall transformers huggingface_hub && UV_PRERELEASE=allow uv pip install -U transformers@git+https://github.com/huggingface/transformers.git + + - name: Environment + run: diffusers-cli env + + - name: Run pytest + env: + HF_TOKEN: ${{ secrets.DIFFUSERS_HF_HUB_READ_TOKEN }} + # https://pytorch.org/docs/stable/notes/randomness.html#avoiding-nondeterministic-algorithms + CUBLAS_WORKSPACE_CONFIG: :16:8 + # Forwarded via env (not interpolated into the script) to avoid breakage on + # quotes/special characters in a legitimate command. + PYTEST_ARGS: ${{ needs.gate.outputs.pytest_args }} + run: | + # Intentionally unquoted so flags/paths in the command word-split as argv. + pytest -n 1 --max-worker-restart=0 --dist=loadfile --make-reports=tests_bot_gpu $PYTEST_ARGS + + - name: Failure short reports + if: ${{ failure() }} + run: | + cat reports/tests_bot_gpu_stats.txt || true + cat reports/tests_bot_gpu_failures_short.txt || true + + - name: Test suite reports artifacts + if: ${{ always() }} + uses: actions/upload-artifact@v6 + with: + name: bot_gpu_test_reports + path: reports + + report: + name: Report status + needs: [gate, gpu_tests] + # Always run so the comment is updated on success, failure, or cancellation β€” + # but only if `gate` actually posted a comment to update. + if: ${{ always() && needs.gate.outputs.comment_id != '' }} + runs-on: ubuntu-22.04 + permissions: + pull-requests: write + steps: + - name: Update comment with final status + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + CID: ${{ needs.gate.outputs.comment_id }} + RESULT: ${{ needs.gpu_tests.result }} + PYTEST_ARGS: ${{ needs.gate.outputs.pytest_args }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + case "$RESULT" in + success) EMOJI="βœ…"; MSG="passed";; + failure) EMOJI="❌"; MSG="failed";; + cancelled) EMOJI="⚠️"; MSG="was cancelled";; + *) EMOJI="⚠️"; MSG="did not run (${RESULT})";; + esac + BODY="${EMOJI} \`pytest ${PYTEST_ARGS}\` ${MSG} on GPU β€” [view logs](${RUN_URL})." + gh api -X PATCH "repos/${REPO}/issues/comments/${CID}" -f body="$BODY" diff --git a/.github/workflows/run_tests_from_a_pr.yml b/.github/workflows/run_tests_from_a_pr.yml deleted file mode 100644 index c1284e12a17d..000000000000 --- a/.github/workflows/run_tests_from_a_pr.yml +++ /dev/null @@ -1,76 +0,0 @@ -name: Check running SLOW tests from a PR (only GPU) - -on: - workflow_dispatch: - inputs: - docker_image: - default: 'diffusers/diffusers-pytorch-cuda' - description: 'Name of the Docker image' - required: true - pr_number: - description: 'PR number to test on' - required: true - test: - description: 'Tests to run (e.g.: `tests/models`).' - required: true - -permissions: - contents: read - -env: - DIFFUSERS_IS_CI: yes - IS_GITHUB_CI: "1" - HF_HOME: /mnt/cache - OMP_NUM_THREADS: 8 - MKL_NUM_THREADS: 8 - PYTEST_TIMEOUT: 600 - RUN_SLOW: yes - -jobs: - run_tests: - name: "Run a test on our runner from a PR" - runs-on: - group: aws-g4dn-2xlarge - container: - image: ${{ github.event.inputs.docker_image }} - options: --gpus all --privileged --ipc host -v /mnt/cache/.cache/huggingface:/mnt/cache/ - - steps: - - name: Validate test files input - id: validate_test_files - env: - PY_TEST: ${{ github.event.inputs.test }} - run: | - if [[ ! "$PY_TEST" =~ ^tests/ ]]; then - echo "Error: The input string must start with 'tests/'." - exit 1 - fi - - if [[ ! "$PY_TEST" =~ ^tests/(models|pipelines|lora) ]]; then - echo "Error: The input string must contain either 'models', 'pipelines', or 'lora' after 'tests/'." - exit 1 - fi - - if [[ "$PY_TEST" == *";"* ]]; then - echo "Error: The input string must not contain ';'." - exit 1 - fi - echo "$PY_TEST" - - shell: bash -e {0} - - - name: Checkout PR branch - uses: actions/checkout@v6 - with: - ref: refs/pull/${{ inputs.pr_number }}/head - - - name: Install pytest - run: | - uv pip install -e ".[quality]" - uv pip install peft - - - name: Run tests - env: - PY_TEST: ${{ github.event.inputs.test }} - run: | - pytest "$PY_TEST" diff --git a/diffusers-bot-gpu-tests-plan.md b/diffusers-bot-gpu-tests-plan.md new file mode 100644 index 000000000000..3bdde0bcd5bc --- /dev/null +++ b/diffusers-bot-gpu-tests-plan.md @@ -0,0 +1,241 @@ +# Plan: `/diffusers-bot` β€” run GPU tests from a PR comment + +## Goal + +Let repository maintainers (admins + anyone with `write` access) kick off GPU tests on a +PR by commenting: + +``` +/diffusers-bot pytest tests/models/test_modeling_common.py -k "some_test" +``` + +The bot then: + +1. Validates the commenter is authorized and the command is well-formed. +2. Triggers a GPU run on the same runner used by `pr_tests_gpu.yml`. +3. Posts a PR comment linking to the Actions run ("⏳ running…"). +4. When the run finishes (success **or** failure), **edits that same comment** with the + final status (βœ… / ❌) and the link. + +Works for PRs from forks (external contributors) as well as same-repo branches. + +--- + +## Key design decisions & constraints + +### 1. Trigger: `issue_comment`, not `pull_request_target` + +We use: + +```yaml +on: + issue_comment: + types: [created] +``` + +`issue_comment` workflows **always run from the workflow file on the repo's default +branch**, in the context of the base repo, with access to base-repo secrets and a +`GITHUB_TOKEN` that can write comments. A fork cannot alter the workflow logic that runs. +This is the correct, safe trigger for a bot command. (`github.event.issue.pull_request` +is set when the comment is on a PR β€” we gate on it.) + +### 2. Authorization β€” must be write/admin, not just author_association + +`claude_review.yml` gates on `author_association` (`MEMBER`/`OWNER`/`COLLABORATOR`). That is +a usable first filter, but `author_association` is **not** a reliable proxy for write +access (e.g. a contributor can be `CONTRIBUTOR` while still being on a team with write). + +The robust check is the collaborator-permission API: + +```bash +PERM=$(gh api "repos/${REPO}/collaborators/${COMMENTER}/permission" --jq '.permission') +# -> one of: admin, write, read, none +[[ "$PERM" == "admin" || "$PERM" == "write" ]] || exit_unauthorized +``` + +We do this in a **gate job** before anything touches the GPU. Unauthorized commenters get a +πŸ‘Ž reaction (or a short comment) and the workflow stops. This is the single most important +security control because the next step **executes untrusted fork code on our GPU runner** β€” +only a trusted maintainer may vouch for that. + +### 3. Command parsing + +**The authorization gate (above) is the trust boundary.** Only admins / write-access +maintainers can trigger this, which is the same trust level as pushing a workflow change or +running CI β€” so we do **not** need an elaborate pytest-arg allowlist or shell-metacharacter +filtering. A maintainer running an arbitrary `pytest` invocation on the runner is +explicitly allowed. + +We keep only basic hygiene, not a security control: + +- Pass the command through an `env:` variable rather than interpolating + `${{ github.event.comment.body }}` straight into a `run:` script. This is to avoid YAML/ + shell breakage on quotes and special characters in a legitimate command β€” not to defend + against the (already-trusted) commenter. +- Parsing: strip the leading `/diffusers-bot pytest` prefix; the remainder is the pytest + argv, forwarded as-is to `pytest`. + +(Optional, purely as a guardrail against typos β€” not security: a soft check that the +command targets `tests/`. Can be dropped if it gets in the way.) + +### 4. Checking out fork PR code + +Resolve the PR number from `github.event.issue.number`, then check out the PR **head** +(works for forks without needing fork credentials), exactly like `run_tests_from_a_pr.yml`: + +```yaml +- uses: actions/checkout@v6 + with: + ref: refs/pull/${{ needs.gate.outputs.pr_number }}/head +``` + +### 5. Secret minimization on the GPU job + +The GPU job runs untrusted code, so it gets the **least** privilege: + +- `permissions: contents: read` (no `pull-requests: write` here). +- Only the secret the tests actually need: `HF_TOKEN: ${{ secrets.DIFFUSERS_HF_HUB_READ_TOKEN }}` + (a **read** token, same as `pr_tests_gpu.yml`). + +All comment creation/editing happens in **separate ubuntu jobs** that never check out fork +code, so the `pull-requests: write` token is never exposed to untrusted code. + +### 6. Comment lifecycle (create once, edit in place) + +- **Gate job** creates the initial comment via `gh api` and captures its id: + ```bash + CID=$(gh api -X POST "repos/${REPO}/issues/${PR}/comments" \ + -f body="⏳ Running \`pytest …\` on GPU β€” [view run]($RUN_URL)" --jq '.id') + echo "comment_id=$CID" >> "$GITHUB_OUTPUT" + ``` + `RUN_URL = ${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}`. +- **Report job** (`needs: [gate, gpu-tests]`, `if: always()`) reads + `needs.gpu-tests.result` (`success`/`failure`/`cancelled`/`skipped`) and **PATCHes** the + same comment id: + ```bash + gh api -X PATCH "repos/${REPO}/issues/comments/${CID}" -f body="$FINAL_BODY" + ``` + +We use `gh api` rather than `peter-evans/create-or-update-comment` to avoid adding a new +third-party action dependency (consistent with how the repo already drives comments via the +`gh` CLI in `claude_review.yml`). + +--- + +## Workflow structure + +New file: `.github/workflows/pr_comment_gpu_tests.yml` + +``` +on: issue_comment (created) + +concurrency: + group: diffusers-bot-${{ github.event.issue.number }} + cancel-in-progress: true # a newer command supersedes an in-flight one for the same PR + +jobs: + gate: # ubuntu, fast + if: PR comment AND body starts with "/diffusers-bot pytest" + permissions: { pull-requests: write } + steps: + - check commenter permission (admin|write) via gh api + - parse pytest args (strip prefix; pass via env, no allowlist needed) + - resolve PR head ref + whether cross-repo (fork) + - πŸ‘€ react to the comment to acknowledge + - create "⏳ running" comment, output comment_id + outputs: { pr_number, pr_ref, pytest_args, comment_id, authorized } + + gpu-tests: # GPU runner, least privilege + needs: gate + if: needs.gate.outputs.authorized == 'true' + runs-on: { group: aws-g4dn-2xlarge } + container: + image: diffusers/diffusers-pytorch-cuda + options: --gpus all --shm-size "16gb" --ipc host + permissions: { contents: read } + env: { HF_TOKEN: secrets.DIFFUSERS_HF_HUB_READ_TOKEN } + steps: + - checkout refs/pull//head + - install deps (see "Dependency installation" below) + - run: pytest $PYTEST_ARGS (args via env; --make-reports for artifacts) + - upload reports artifact (if: always()) + + report: # ubuntu, edits the comment + needs: [gate, gpu-tests] + if: always() && needs.gate.outputs.comment_id != '' + permissions: { pull-requests: write } + steps: + - compute status from needs.gpu-tests.result + - PATCH the comment_id with βœ…/❌/⚠️ + run link (+ short failure hint) +``` + +--- + +## Dependency installation (GPU job) + +Mirror `pr_tests_gpu.yml`'s `torch_cuda_tests` install block, but pull in **all** the +training/test extras from `setup.py` so peft and the rest are present: + +```bash +printf 'tokenizers<0.23.0\ntorch==2.10.0\ntorchvision==0.25.0\ntorchaudio==2.10.0\n' > "$UV_OVERRIDE" +uv pip install -e ".[quality,training,test]" # training -> peft, accelerate, datasets, timm, … + # test -> pytest, pytest-xdist, parameterized, … +uv pip install peft@git+https://github.com/huggingface/peft.git +uv pip uninstall accelerate && uv pip install -U accelerate@git+https://github.com/huggingface/accelerate.git +uv pip uninstall transformers huggingface_hub && UV_PRERELEASE=allow uv pip install -U transformers@git+https://github.com/huggingface/transformers.git +``` + +- `[training]` provides peft + the training stack; `[test]` provides the pytest toolchain β€” + both as declared in `setup.py`. +- The git installs of peft/accelerate/transformers (and the `UV_OVERRIDE` pin) match + `pr_tests_gpu.yml` so the environment is identical to the existing GPU PR tests. + +--- + +## Edge cases handled + +- **Comment is not on a PR** β†’ `github.event.issue.pull_request` is null β†’ job skipped. +- **Comment doesn't start with the command** β†’ `if:` filters it out (no run spent). +- **Unauthorized commenter** β†’ gate fails fast and posts a reply comment ("not authorized + to run this"), no GPU time used. +- **Fork PR** β†’ handled by `refs/pull//head` checkout; fork code never sees the + comment-write token. +- **Run cancelled / superseded** (concurrency) β†’ report job's `always()` patches the + comment to ⚠️ cancelled. +- **Test job genuinely fails** β†’ comment edited to ❌ with link to logs + reports artifact. + +--- + +## Security summary (why this is safe to run fork code on a GPU) + +| Risk | Mitigation | +|------|-----------| +| Fork modifies the bot logic | `issue_comment` runs the default-branch workflow file only | +| Untrusted user triggers GPU/secret use | hard `admin`/`write` permission check via API β€” **this is the trust boundary** | +| Untrusted code exfiltrates write token | GPU job has `contents: read` only; commenting isolated to separate jobs | +| Untrusted code abuses powerful secrets | only a **read** HF token is exposed, same as existing GPU PR tests | + +The command string itself is **not** treated as an attack surface: only trusted +maintainers can issue it, so arbitrary `pytest` args are by design. + +--- + +## Deliverables / implementation steps + +1. Add `.github/workflows/pr_comment_gpu_tests.yml` implementing the 3-job structure above. +2. Reuse the dependency-install + override-pin steps per "Dependency installation" so the + env matches `pr_tests_gpu.yml`. +3. **Remove `.github/workflows/run_tests_from_a_pr.yml`** β€” this comment-driven workflow + supersedes that `workflow_dispatch`-based one. +4. (Optional follow-up) Document the command in `CONTRIBUTING`/maintainer notes: usage and + who can run it. + +### Resolved decisions + +- **Command prefix**: `/diffusers-bot pytest …` exactly. +- **Allowed test scope**: any `pytest` invocation (trusted maintainer) β€” no allowlist. +- **Runner**: `aws-g4dn-2xlarge` (single GPU, matches existing PR GPU job). +- **Acknowledgement**: πŸ‘€ reaction on the triggering comment, plus the "⏳ running" comment. +- **Dependencies**: install `.[quality,training,test]` (peft + all training/test extras from + `setup.py`) plus the git installs from `pr_tests_gpu.yml`. +``` From 893ac3b87fd36274a0b1f174144ffaf346bb18d9 Mon Sep 17 00:00:00 2001 From: sayakpaul Date: Fri, 5 Jun 2026 15:48:36 +0530 Subject: [PATCH 2/3] fix --- .github/workflows/pr_comment_gpu_tests.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/pr_comment_gpu_tests.yml b/.github/workflows/pr_comment_gpu_tests.yml index d3f159653cae..377624e26e11 100644 --- a/.github/workflows/pr_comment_gpu_tests.yml +++ b/.github/workflows/pr_comment_gpu_tests.yml @@ -154,8 +154,7 @@ jobs: # quotes/special characters in a legitimate command. PYTEST_ARGS: ${{ needs.gate.outputs.pytest_args }} run: | - # Intentionally unquoted so flags/paths in the command word-split as argv. - pytest -n 1 --max-worker-restart=0 --dist=loadfile --make-reports=tests_bot_gpu $PYTEST_ARGS + eval "pytest --make-reports=tests_bot_gpu $PYTEST_ARGS" - name: Failure short reports if: ${{ failure() }} From f1312c55a03f926efb46a2a3f628fa01ece55f92 Mon Sep 17 00:00:00 2001 From: sayakpaul Date: Fri, 5 Jun 2026 16:19:10 +0530 Subject: [PATCH 3/3] remove planning --- diffusers-bot-gpu-tests-plan.md | 241 -------------------------------- 1 file changed, 241 deletions(-) delete mode 100644 diffusers-bot-gpu-tests-plan.md diff --git a/diffusers-bot-gpu-tests-plan.md b/diffusers-bot-gpu-tests-plan.md deleted file mode 100644 index 3bdde0bcd5bc..000000000000 --- a/diffusers-bot-gpu-tests-plan.md +++ /dev/null @@ -1,241 +0,0 @@ -# Plan: `/diffusers-bot` β€” run GPU tests from a PR comment - -## Goal - -Let repository maintainers (admins + anyone with `write` access) kick off GPU tests on a -PR by commenting: - -``` -/diffusers-bot pytest tests/models/test_modeling_common.py -k "some_test" -``` - -The bot then: - -1. Validates the commenter is authorized and the command is well-formed. -2. Triggers a GPU run on the same runner used by `pr_tests_gpu.yml`. -3. Posts a PR comment linking to the Actions run ("⏳ running…"). -4. When the run finishes (success **or** failure), **edits that same comment** with the - final status (βœ… / ❌) and the link. - -Works for PRs from forks (external contributors) as well as same-repo branches. - ---- - -## Key design decisions & constraints - -### 1. Trigger: `issue_comment`, not `pull_request_target` - -We use: - -```yaml -on: - issue_comment: - types: [created] -``` - -`issue_comment` workflows **always run from the workflow file on the repo's default -branch**, in the context of the base repo, with access to base-repo secrets and a -`GITHUB_TOKEN` that can write comments. A fork cannot alter the workflow logic that runs. -This is the correct, safe trigger for a bot command. (`github.event.issue.pull_request` -is set when the comment is on a PR β€” we gate on it.) - -### 2. Authorization β€” must be write/admin, not just author_association - -`claude_review.yml` gates on `author_association` (`MEMBER`/`OWNER`/`COLLABORATOR`). That is -a usable first filter, but `author_association` is **not** a reliable proxy for write -access (e.g. a contributor can be `CONTRIBUTOR` while still being on a team with write). - -The robust check is the collaborator-permission API: - -```bash -PERM=$(gh api "repos/${REPO}/collaborators/${COMMENTER}/permission" --jq '.permission') -# -> one of: admin, write, read, none -[[ "$PERM" == "admin" || "$PERM" == "write" ]] || exit_unauthorized -``` - -We do this in a **gate job** before anything touches the GPU. Unauthorized commenters get a -πŸ‘Ž reaction (or a short comment) and the workflow stops. This is the single most important -security control because the next step **executes untrusted fork code on our GPU runner** β€” -only a trusted maintainer may vouch for that. - -### 3. Command parsing - -**The authorization gate (above) is the trust boundary.** Only admins / write-access -maintainers can trigger this, which is the same trust level as pushing a workflow change or -running CI β€” so we do **not** need an elaborate pytest-arg allowlist or shell-metacharacter -filtering. A maintainer running an arbitrary `pytest` invocation on the runner is -explicitly allowed. - -We keep only basic hygiene, not a security control: - -- Pass the command through an `env:` variable rather than interpolating - `${{ github.event.comment.body }}` straight into a `run:` script. This is to avoid YAML/ - shell breakage on quotes and special characters in a legitimate command β€” not to defend - against the (already-trusted) commenter. -- Parsing: strip the leading `/diffusers-bot pytest` prefix; the remainder is the pytest - argv, forwarded as-is to `pytest`. - -(Optional, purely as a guardrail against typos β€” not security: a soft check that the -command targets `tests/`. Can be dropped if it gets in the way.) - -### 4. Checking out fork PR code - -Resolve the PR number from `github.event.issue.number`, then check out the PR **head** -(works for forks without needing fork credentials), exactly like `run_tests_from_a_pr.yml`: - -```yaml -- uses: actions/checkout@v6 - with: - ref: refs/pull/${{ needs.gate.outputs.pr_number }}/head -``` - -### 5. Secret minimization on the GPU job - -The GPU job runs untrusted code, so it gets the **least** privilege: - -- `permissions: contents: read` (no `pull-requests: write` here). -- Only the secret the tests actually need: `HF_TOKEN: ${{ secrets.DIFFUSERS_HF_HUB_READ_TOKEN }}` - (a **read** token, same as `pr_tests_gpu.yml`). - -All comment creation/editing happens in **separate ubuntu jobs** that never check out fork -code, so the `pull-requests: write` token is never exposed to untrusted code. - -### 6. Comment lifecycle (create once, edit in place) - -- **Gate job** creates the initial comment via `gh api` and captures its id: - ```bash - CID=$(gh api -X POST "repos/${REPO}/issues/${PR}/comments" \ - -f body="⏳ Running \`pytest …\` on GPU β€” [view run]($RUN_URL)" --jq '.id') - echo "comment_id=$CID" >> "$GITHUB_OUTPUT" - ``` - `RUN_URL = ${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}`. -- **Report job** (`needs: [gate, gpu-tests]`, `if: always()`) reads - `needs.gpu-tests.result` (`success`/`failure`/`cancelled`/`skipped`) and **PATCHes** the - same comment id: - ```bash - gh api -X PATCH "repos/${REPO}/issues/comments/${CID}" -f body="$FINAL_BODY" - ``` - -We use `gh api` rather than `peter-evans/create-or-update-comment` to avoid adding a new -third-party action dependency (consistent with how the repo already drives comments via the -`gh` CLI in `claude_review.yml`). - ---- - -## Workflow structure - -New file: `.github/workflows/pr_comment_gpu_tests.yml` - -``` -on: issue_comment (created) - -concurrency: - group: diffusers-bot-${{ github.event.issue.number }} - cancel-in-progress: true # a newer command supersedes an in-flight one for the same PR - -jobs: - gate: # ubuntu, fast - if: PR comment AND body starts with "/diffusers-bot pytest" - permissions: { pull-requests: write } - steps: - - check commenter permission (admin|write) via gh api - - parse pytest args (strip prefix; pass via env, no allowlist needed) - - resolve PR head ref + whether cross-repo (fork) - - πŸ‘€ react to the comment to acknowledge - - create "⏳ running" comment, output comment_id - outputs: { pr_number, pr_ref, pytest_args, comment_id, authorized } - - gpu-tests: # GPU runner, least privilege - needs: gate - if: needs.gate.outputs.authorized == 'true' - runs-on: { group: aws-g4dn-2xlarge } - container: - image: diffusers/diffusers-pytorch-cuda - options: --gpus all --shm-size "16gb" --ipc host - permissions: { contents: read } - env: { HF_TOKEN: secrets.DIFFUSERS_HF_HUB_READ_TOKEN } - steps: - - checkout refs/pull//head - - install deps (see "Dependency installation" below) - - run: pytest $PYTEST_ARGS (args via env; --make-reports for artifacts) - - upload reports artifact (if: always()) - - report: # ubuntu, edits the comment - needs: [gate, gpu-tests] - if: always() && needs.gate.outputs.comment_id != '' - permissions: { pull-requests: write } - steps: - - compute status from needs.gpu-tests.result - - PATCH the comment_id with βœ…/❌/⚠️ + run link (+ short failure hint) -``` - ---- - -## Dependency installation (GPU job) - -Mirror `pr_tests_gpu.yml`'s `torch_cuda_tests` install block, but pull in **all** the -training/test extras from `setup.py` so peft and the rest are present: - -```bash -printf 'tokenizers<0.23.0\ntorch==2.10.0\ntorchvision==0.25.0\ntorchaudio==2.10.0\n' > "$UV_OVERRIDE" -uv pip install -e ".[quality,training,test]" # training -> peft, accelerate, datasets, timm, … - # test -> pytest, pytest-xdist, parameterized, … -uv pip install peft@git+https://github.com/huggingface/peft.git -uv pip uninstall accelerate && uv pip install -U accelerate@git+https://github.com/huggingface/accelerate.git -uv pip uninstall transformers huggingface_hub && UV_PRERELEASE=allow uv pip install -U transformers@git+https://github.com/huggingface/transformers.git -``` - -- `[training]` provides peft + the training stack; `[test]` provides the pytest toolchain β€” - both as declared in `setup.py`. -- The git installs of peft/accelerate/transformers (and the `UV_OVERRIDE` pin) match - `pr_tests_gpu.yml` so the environment is identical to the existing GPU PR tests. - ---- - -## Edge cases handled - -- **Comment is not on a PR** β†’ `github.event.issue.pull_request` is null β†’ job skipped. -- **Comment doesn't start with the command** β†’ `if:` filters it out (no run spent). -- **Unauthorized commenter** β†’ gate fails fast and posts a reply comment ("not authorized - to run this"), no GPU time used. -- **Fork PR** β†’ handled by `refs/pull//head` checkout; fork code never sees the - comment-write token. -- **Run cancelled / superseded** (concurrency) β†’ report job's `always()` patches the - comment to ⚠️ cancelled. -- **Test job genuinely fails** β†’ comment edited to ❌ with link to logs + reports artifact. - ---- - -## Security summary (why this is safe to run fork code on a GPU) - -| Risk | Mitigation | -|------|-----------| -| Fork modifies the bot logic | `issue_comment` runs the default-branch workflow file only | -| Untrusted user triggers GPU/secret use | hard `admin`/`write` permission check via API β€” **this is the trust boundary** | -| Untrusted code exfiltrates write token | GPU job has `contents: read` only; commenting isolated to separate jobs | -| Untrusted code abuses powerful secrets | only a **read** HF token is exposed, same as existing GPU PR tests | - -The command string itself is **not** treated as an attack surface: only trusted -maintainers can issue it, so arbitrary `pytest` args are by design. - ---- - -## Deliverables / implementation steps - -1. Add `.github/workflows/pr_comment_gpu_tests.yml` implementing the 3-job structure above. -2. Reuse the dependency-install + override-pin steps per "Dependency installation" so the - env matches `pr_tests_gpu.yml`. -3. **Remove `.github/workflows/run_tests_from_a_pr.yml`** β€” this comment-driven workflow - supersedes that `workflow_dispatch`-based one. -4. (Optional follow-up) Document the command in `CONTRIBUTING`/maintainer notes: usage and - who can run it. - -### Resolved decisions - -- **Command prefix**: `/diffusers-bot pytest …` exactly. -- **Allowed test scope**: any `pytest` invocation (trusted maintainer) β€” no allowlist. -- **Runner**: `aws-g4dn-2xlarge` (single GPU, matches existing PR GPU job). -- **Acknowledgement**: πŸ‘€ reaction on the triggering comment, plus the "⏳ running" comment. -- **Dependencies**: install `.[quality,training,test]` (peft + all training/test extras from - `setup.py`) plus the git installs from `pr_tests_gpu.yml`. -```