Skip to content

pr-states: fix fork-PR token + add run-ci label awareness - #25392

Merged
hnyls2002 merged 7 commits into
sgl-project:mainfrom
hnyls2002:lsyin/fix-awareness-fork-pr
May 15, 2026
Merged

hnyls2002 merged 7 commits into
sgl-project:mainfrom
hnyls2002:lsyin/fix-awareness-fork-pr

Conversation

@hnyls2002

@hnyls2002 hnyls2002 commented May 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Switch pr-states.yml trigger from pull_request to pull_request_target so fork PRs receive a write-enabled GITHUB_TOKEN and the awareness comment update succeeds
  • Show a red ❌ when the PR is missing the run-ci label (parallel to the existing run-ci-extra warning)
  • Drop a redundant 4-line code comment in slash_command_handler.py (the deprecation message posted right below already states the rationale)

Background — fork PR token

  • On pull_request events from a fork, GitHub forces GITHUB_TOKEN to be read-only — this is a platform-level security policy that ignores the permissions: pull-requests: write declaration in the workflow file
  • pr-states / update-pr-body calls PATCH /repos/.../pulls/<n> which fails with 403 Resource not accessible by integration on every fork PR (reproduced on this PR's first run before the fix: Run 25913505322)
  • 100% of fork PRs end up with a red pr-states / update-pr-body check in their checks list, which is confusing for external contributors

Fix — pull_request_target

  • pull_request_target runs in the context of the base branch and grants the workflow a write-enabled token, so the API PATCH succeeds for both fork and non-fork PRs
  • This workflow does not checkout PR head code — it only reads PR metadata via the GitHub API and PATCHes the PR body. The standard pull_request_target supply-chain concern (running untrusted fork code with a write token) does not apply

run-ci label awareness

  • Without the run-ci label, pr-test.yml's gate (pr-gate.yml:57) fails and all downstream stages skip — but the awareness block previously linked to the (effectively empty) run, which was misleading
  • Now mirrors the run-ci-extra pattern: show :x: **Missing run-ci label** instead of a stale link

Verification

  • The pull_request_target switch cannot be observed on this PR's CI: GitHub runs the workflow file from the base branch, so until this is merged the fork PR still hits 403. The fix is reviewable as code; merge will activate it for the next fork PR

Follows up on #25387.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Note

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

@hnyls2002 hnyls2002 changed the title tweak pr-states comment pr-states: use pull_request_target so fork PRs get write token May 15, 2026
@hnyls2002 hnyls2002 changed the title pr-states: use pull_request_target so fork PRs get write token pr-states: fix fork-PR token + add run-ci label awareness May 15, 2026
@hnyls2002

hnyls2002 commented May 15, 2026

Copy link
Copy Markdown
Collaborator Author

/tag-and-rerun-ci

@hnyls2002
hnyls2002 merged commit 293027a into sgl-project:main May 15, 2026
117 of 130 checks passed
@hnyls2002
hnyls2002 deleted the lsyin/fix-awareness-fork-pr branch May 15, 2026 11:10
Chronostasys pushed a commit to MindLab-Research/sglang that referenced this pull request Aug 24, 2026
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.

1 participant