From cd92db8eaae0125179b765c5336bff9f9177d3cf Mon Sep 17 00:00:00 2001 From: Kanghwan Jang <861393+karljang@users.noreply.github.com> Date: Wed, 9 Sep 2026 00:36:12 -0700 Subject: [PATCH 1/6] [None][infra] Rate limit new contributor PR submissions Signed-off-by: Kanghwan Jang <861393+karljang@users.noreply.github.com> --- .github/scripts/pr_rate_limit.test.js | 163 ++++++++++++++++++++++++++ .github/workflows/pr-rate-limit.yml | 134 +++++++++++++++++++++ .github/workflows/precommit-check.yml | 3 + AGENTS.md | 4 + CONTRIBUTING.md | 13 ++ 5 files changed, 317 insertions(+) create mode 100644 .github/scripts/pr_rate_limit.test.js create mode 100644 .github/workflows/pr-rate-limit.yml diff --git a/.github/scripts/pr_rate_limit.test.js b/.github/scripts/pr_rate_limit.test.js new file mode 100644 index 000000000000..f67806f8c617 --- /dev/null +++ b/.github/scripts/pr_rate_limit.test.js @@ -0,0 +1,163 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const { test } = require('node:test'); +const workflow = fs.readFileSync(`${__dirname}/../workflows/pr-rate-limit.yml`, 'utf8'); +const script = workflow.split(' script: |\n')[1] + .split('\n').map((line) => line.replace(/^ {12}/, '')).join('\n'); +const execute = new (Object.getPrototypeOf(async function () {}).constructor)( + 'github', 'context', 'core', 'process', 'Date', script, +); +const now = Date.parse('2026-09-09T12:00:00Z'); +const hour = 3600000; +function pr(number, overrides = {}) { + return { number, created_at: new Date(now - hour + number * 1000).toISOString(), + state: 'open', labels: [], user: { id: 42, login: 'new-user', type: 'User' }, + pull_request: {}, ...overrides }; +} +async function run(options = {}) { + const current = options.current || pr(6); + const writes = options.writes || []; + const calls = []; + const github = { rest: { + pulls: { + get: async ({ pull_number }) => { + calls.push(pull_number); + if (options.readError) throw new Error('API failure'); + return { data: pull_number === current.number + ? (calls.filter((n) => n === current.number).length > 1 && options.recheck || current) + : { ...pr(pull_number), merged_at: options.merged ? '2026-09-08T00:00:00Z' : null } }; + }, + update: async (args) => { + if (options.closeError) throw new Error('close failed'); + writes.push({ kind: 'close', ...args }); + }, + }, + repos: { getCollaboratorPermissionLevel: async () => { + if (options.permissionError) throw new Error('permission failed'); + return { data: { permission: options.permission || 'read', user: { permissions: { push: options.push } } } }; + } }, + issues: { + listForRepo: async (args) => { + assert.equal(args.creator, current.user.login); + assert.equal(args.state, 'all'); + if (options.historyError || args.page === options.failedPage) throw new Error('history failed'); + return { data: options.pages ? (options.pages[args.page - 1] || []) + : options.history || Array.from({ length: 6 }, (_, i) => pr(i + 1)) }; + }, + listComments: () => {}, + createComment: async (args) => { + if (options.commentError) throw new Error('comment failed'); + writes.push({ kind: 'comment', ...args }); + }, + }, + }, paginate: async () => { + if (options.commentsError) throw new Error('comments failed'); + return options.comments || []; + } }; + const context = { repo: { owner: 'NVIDIA', repo: 'TensorRT-LLM' }, payload: { pull_request: current } }; + await execute(github, context, { info: () => {} }, { env: options.env || {} }, + class extends Date { static now() { return now; } }); + return { writes, calls }; +} +test('first five pass; sixth receives explanation before closure', async () => { + for (let n = 1; n <= 5; n++) assert.deepEqual((await run({ current: pr(n) })).writes, []); + const { writes } = await run(); + assert.deepEqual(writes.map((w) => w.kind), ['comment', 'close']); + assert.match(writes[0].body, /submitted 6 PRs/); + assert.match(writes[0].body, /no merged PRs/); + assert.match(writes[0].body, /2026-09-10T11:00:06.000Z/); + assert.equal(writes[1].state, 'closed'); +}); +test('counts drafts and closed unmerged PRs, but not ordinary issues or another author', async () => { + const history = Array.from({ length: 5 }, (_, i) => pr(i + 1, { draft: true, state: 'closed' })); + history.push(pr(0, { pull_request: undefined }), pr(-1, { user: { id: 99 } })); + assert.equal((await run({ history })).writes.length, 2); +}); +test('exact rolling boundary excluded; later PRs cannot penalize earlier submissions', async () => { + const current = pr(6); + const boundary = new Date(Date.parse(current.created_at) - 24 * hour).toISOString(); + const history = [pr(1, { created_at: boundary }), ...[2, 3, 4, 5].map((n) => pr(n)), pr(7)]; + assert.deepEqual((await run({ current, history })).writes, []); +}); +test('same-second bursts ordered by PR number, regardless of execution order', async () => { + const history = Array.from({ length: 10 }, (_, i) => pr(i + 1, { created_at: pr(1).created_at })); + assert.deepEqual((await run({ current: history[4], history })).writes, []); + assert.equal((await run({ current: history[5], history })).writes.length, 2); +}); +test('history pagination and duplicate entries do not change quota', async () => { + const first = Array.from({ length: 100 }, () => pr(1)); + const { writes } = await run({ pages: [first, [pr(2), pr(3), pr(4), pr(5)]] }); + assert.equal(writes.length, 2); + assert.match(writes[0].body, /submitted 6 PRs/); +}); +test('actual merged history exempts; author association alone does not', async () => { + const history = Array.from({ length: 6 }, (_, i) => pr(i + 1)); + history.push(pr(0, { state: 'closed' })); + assert.deepEqual((await run({ history, merged: true })).writes, []); + assert.equal((await run({ current: pr(6, { author_association: 'CONTRIBUTOR' }) })).writes.length, 2); +}); +test('maintainers, bots, allowlisted users and labeled PRs are exempt', async () => { + for (const permission of ['write', 'admin', 'maintain']) + assert.deepEqual((await run({ permission })).writes, []); + assert.deepEqual((await run({ env: { EXEMPT_USERS: 'other, NEW-USER ' } })).writes, []); + assert.deepEqual((await run({ push: true })).writes, []); + assert.deepEqual((await run({ current: pr(6, { user: { id: 42, login: 'bot', type: 'Bot' } }) })).writes, []); + assert.deepEqual((await run({ current: pr(6, { labels: [{ name: 'pr-rate-limit-exempt' }] }) })).writes, []); +}); +test('dry run, closed PRs, and exemptions added during evaluation make no writes', async () => { + assert.deepEqual((await run({ env: { DRY_RUN: 'true' } })).writes, []); + assert.deepEqual((await run({ current: pr(6, { state: 'closed' }) })).writes, []); + assert.deepEqual((await run({ recheck: pr(6, { labels: [{ name: 'pr-rate-limit-exempt' }] }) })).writes, []); + assert.deepEqual((await run({ recheck: pr(6, { state: 'closed' }) })).writes, []); +}); +test('reopening after cooldown is allowed; reopening before it is still limited', async () => { + assert.deepEqual((await run({ current: pr(6, { created_at: new Date(now - 24 * hour).toISOString() }) })).writes, []); + assert.equal((await run()).writes.length, 2); +}); +test('reruns reuse only bot-authored comments and retry closure', async () => { + const body = ''; + const trusted = { user: { login: 'github-actions[bot]' }, body }; + assert.deepEqual((await run({ comments: [trusted] })).writes.map((w) => w.kind), ['close']); + assert.equal((await run({ comments: [{ user: { login: 'new-user' }, body }] })).writes.length, 2); +}); +test('API failures, incomplete history and invalid timestamps abort without moderation', async () => { + for (const options of [{ readError: true }, { historyError: true }, { commentError: true }, + { permissionError: true }, { commentsError: true }, + { pages: [Array.from({ length: 100 }, () => pr(1))], failedPage: 2 }, + { history: [pr(1, { created_at: 'invalid' })] }, + { current: pr(6, { created_at: 'invalid' }) }, + { pages: Array.from({ length: 20 }, () => Array.from({ length: 100 }, () => pr(1))) }]) { + const writes = []; + await assert.rejects(run({ ...options, writes })); + assert.deepEqual(writes, []); + } +}); +test('successful comment survives a close failure and is not duplicated on retry', async () => { + const writes = []; + await assert.rejects(run({ closeError: true, writes }), /close failed/); + assert.deepEqual(writes.map((w) => w.kind), ['comment']); + const comments = [{ user: { login: 'github-actions[bot]' }, body: writes[0].body }]; + assert.deepEqual((await run({ comments })).writes.map((w) => w.kind), ['close']); +}); +test('workflow uses trusted inline code, minimal permissions and per-PR concurrency', () => { + assert.match(workflow, /pull_request_target:/); + assert.match(workflow, /types: \[opened, reopened, ready_for_review\]/); + assert.match(workflow, /pull-requests: write/); + assert.match(workflow, /issues: read/); + assert.match(workflow, /group: pr-rate-limit-\$\{\{ github.event.pull_request.number \}\}/); + assert.doesNotMatch(workflow, /actions\/checkout|head.sha|secrets\./); +}); diff --git a/.github/workflows/pr-rate-limit.yml b/.github/workflows/pr-rate-limit.yml new file mode 100644 index 000000000000..42d93da1f64b --- /dev/null +++ b/.github/workflows/pr-rate-limit.yml @@ -0,0 +1,134 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: New Contributor PR Rate Limit + +on: + pull_request_target: + types: [opened, reopened, ready_for_review] + +permissions: + contents: read + pull-requests: write + issues: read + +# Serialize retries for one PR, not all submissions by an author: a shared +# author group can discard pending runs during the very burst we must handle. +concurrency: + group: pr-rate-limit-${{ github.event.pull_request.number }} + cancel-in-progress: false + +jobs: + rate-limit: + if: github.repository == 'NVIDIA/TensorRT-LLM' + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + # No checkout or execution of PR code in this privileged workflow. + # Administrators can set PR_RATE_LIMIT_DRY_RUN=true to disable writes. + # PR_RATE_LIMIT_EXEMPT_USERS is a comma-separated list of trusted logins. + - uses: actions/github-script@v8 + env: + DRY_RUN: ${{ vars.PR_RATE_LIMIT_DRY_RUN }} + EXEMPT_USERS: ${{ vars.PR_RATE_LIMIT_EXEMPT_USERS }} + with: + script: | + const LIMIT = 5; + const WINDOW_MS = 24 * 60 * 60 * 1000; + const EXEMPT_LABEL = 'pr-rate-limit-exempt'; + const MARKER = ''; + const { owner, repo } = context.repo; + const pull_number = context.payload.pull_request.number; + const now = Date.now(); + const getPull = async (number) => (await github.rest.pulls.get({ + owner, repo, pull_number: number, + })).data; + const pull = await getPull(pull_number); + const created = Date.parse(pull.created_at); + if (!Number.isFinite(created) || created > now) { + throw new Error('Invalid pull request creation time; no moderation performed.'); + } + // Reopening does not constitute a new submission. Once the original + // submission is 24 hours old, the contributor may reopen it for review. + if (pull.state !== 'open' || now >= created + WINDOW_MS) return; + const login = pull.user.login; + const exemptUsers = (process.env.EXEMPT_USERS || '').split(',') + .map((value) => value.trim().toLowerCase()).filter(Boolean); + const hasExemptLabel = (pr) => pr.labels.some((label) => label.name === EXEMPT_LABEL); + if (pull.user.type === 'Bot' || exemptUsers.includes(login.toLowerCase()) || hasExemptLabel(pull)) return; + const { data: permission } = await github.rest.repos.getCollaboratorPermissionLevel({ + owner, repo, username: login, + }); + if (permission.user?.permissions?.push || ['admin', 'write', 'maintain'].includes(permission.permission)) return; + + // The creator-filtered issues API includes PRs in every state, without + // relying on the eventually consistent search index. Complete the listing + // before making any changes. Excessive history or API failures abort safely. + const history = new Map(); + let complete = false; + for (let page = 1; page <= 20; page += 1) { + const { data } = await github.rest.issues.listForRepo({ + owner, repo, creator: login, state: 'all', sort: 'created', + direction: 'desc', per_page: 100, page, + }); + for (const item of data) { + if (item.pull_request && item.user.id === pull.user.id) history.set(item.number, item); + } + if (data.length < 100) { complete = true; break; } + } + if (!complete) throw new Error('Contributor history exceeded 2000 items; no moderation performed.'); + // Include the triggering PR even if its listing has not caught up yet. + history.set(pull.number, pull); + const preceding = [...history.values()].filter((item) => { + const time = Date.parse(item.created_at); + if (!Number.isFinite(time)) throw new Error('Invalid history timestamp; no moderation performed.'); + return time > created - WINDOW_MS && + (time < created || (time === created && item.number <= pull.number)); + }); + if (preceding.length <= LIMIT) return; + // A closed PR is not necessarily a contribution: require an actual merge. + // Check history directly rather than trusting author_association or a + // search result that may not yet include a recent merge. + for (const item of history.values()) { + if (item.state === 'closed') { + const previous = await getPull(item.number); + if (previous.merged_at) return; + } + } + const body = [ + 'You submitted ' + preceding.length + ' PRs in the 24-hour window ending with this submission. ' + + 'Contributors with no merged PRs in this repository are limited to ' + LIMIT + ' submissions per rolling 24 hours, including drafts and closed PRs.', + 'Closing this PR because it exceeds that limit. Please consolidate related changes into coherent, validated PRs. ' + + 'You may reopen this PR after ' + new Date(created + WINDOW_MS).toISOString() + + ', or ask a maintainer for an exception.', + MARKER, + ].join('\n\n'); + core.info('PR #' + pull_number + ': ' + preceding.length + ' submissions; limit ' + LIMIT); + if (process.env.DRY_RUN === 'true') { + core.info('Dry run: would comment and close.'); + return; + } + const comments = await github.paginate(github.rest.issues.listComments, { + owner, repo, issue_number: pull_number, per_page: 100, + }); + // Recheck after pagination, immediately before writing. + const current = await getPull(pull_number); + if (current.state !== 'open' || hasExemptLabel(current) || Date.now() >= created + WINDOW_MS) return; + if (!comments.some((comment) => comment.user?.login === 'github-actions[bot]' && comment.body?.includes(MARKER))) { + // Do not blindly retry a comment write: a timeout may follow a successful + // write. A rerun reconciles against the bot-authored marker above. + await github.rest.issues.createComment({ owner, repo, issue_number: pull_number, body }); + } + await github.rest.pulls.update({ owner, repo, pull_number, state: 'closed' }); diff --git a/.github/workflows/precommit-check.yml b/.github/workflows/precommit-check.yml index 3e68e6df598d..17165b6247db 100644 --- a/.github/workflows/precommit-check.yml +++ b/.github/workflows/precommit-check.yml @@ -36,6 +36,9 @@ jobs: - name: Test stale pull request cleanup workflow run: node .github/scripts/cleanup_stale_prs.test.js + - name: Test contributor PR rate limit workflow + run: node --test .github/scripts/pr_rate_limit.test.js + - uses: actions/setup-python@v6 with: python-version: '3.12' diff --git a/AGENTS.md b/AGENTS.md index abc5d75a6bca..eb06ee73213f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -167,6 +167,10 @@ See [CI overview](docs/source/developer-guide/ci-overview.md) for full details. ### Triggering CI +The new-contributor submission limit is implemented in `.github/workflows/pr-rate-limit.yml`; +see `CONTRIBUTING.md` for the five-PR/24-hour policy and maintainer exemptions. Its mocked API tests +run with `node --test .github/scripts/pr_rate_limit.test.js` and are registered in Release Checks. + CI is triggered by posting comments on the PR. Basic commands: - `/bot run` — trigger the standard CI pipeline - `/bot run --disable-fail-fast` — run all stages even if earlier ones fail (only add when explicitly needed) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 866b13438f08..bc023e051de3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -118,6 +118,19 @@ For NVIDIA developers, please submit feature or bug fixes to the dedicated bran Meanwhile, please add the "release blocker" label to any PRs that could potentially cause a release delay. +### Submission rate for new contributors + +Contributors with no merged PRs in this repository may submit up to five PRs per rolling 24 hours. +Drafts and closed PRs count toward this limit. Please consolidate related changes into coherent, +validated PRs. Excess submissions receive an explanation and are automatically closed after creation. +The comment gives a time, 24 hours after that PR's creation, when it may be reopened for review. + +Users with write access and bot accounts are exempt. Maintainers can apply `pr-rate-limit-exempt` +to an individual PR before reopening it. Administrators can exempt trusted contributors through the +comma-separated `PR_RATE_LIMIT_EXEMPT_USERS` repository variable, or set `PR_RATE_LIMIT_DRY_RUN=true` +to log decisions without commenting or closing. The workflow does not prevent independently triggered +CI jobs from starting. If GitHub history cannot be read completely, it fails without closing the PR. + ### Inactive pull requests TensorRT LLM automatically reviews inactive pull requests each day. This policy applies to both draft and From f956c4c5acdc259b0c65be334267167de1d541a0 Mon Sep 17 00:00:00 2001 From: Kanghwan Jang <861393+karljang@users.noreply.github.com> Date: Wed, 9 Sep 2026 12:36:45 -0700 Subject: [PATCH 2/6] Enforce open PR cap and harden contributor limit recovery Signed-off-by: Kanghwan Jang <861393+karljang@users.noreply.github.com> --- .github/scripts/pr_rate_limit.test.js | 90 ++++++++++++++++++++---- .github/workflows/pr-rate-limit.yml | 98 +++++++++++++++++---------- AGENTS.md | 2 +- CONTRIBUTING.md | 18 +++-- 4 files changed, 155 insertions(+), 53 deletions(-) diff --git a/.github/scripts/pr_rate_limit.test.js b/.github/scripts/pr_rate_limit.test.js index f67806f8c617..93f04ad0abab 100644 --- a/.github/scripts/pr_rate_limit.test.js +++ b/.github/scripts/pr_rate_limit.test.js @@ -53,8 +53,12 @@ async function run(options = {}) { issues: { listForRepo: async (args) => { assert.equal(args.creator, current.user.login); - assert.equal(args.state, 'all'); + assert.ok(['all', 'open'].includes(args.state)); if (options.historyError || args.page === options.failedPage) throw new Error('history failed'); + if (args.state === 'open' && options.openHistoryError) throw new Error('open history failed'); + if (args.state === 'open') return { data: args.page === 1 + ? options.openHistory || (options.history || Array.from({ length: 6 }, (_, i) => pr(i + 1))) + .filter((item) => item.state === 'open') : [] }; return { data: options.pages ? (options.pages[args.page - 1] || []) : options.history || Array.from({ length: 6 }, (_, i) => pr(i + 1)) }; }, @@ -68,13 +72,13 @@ async function run(options = {}) { if (options.commentsError) throw new Error('comments failed'); return options.comments || []; } }; - const context = { repo: { owner: 'NVIDIA', repo: 'TensorRT-LLM' }, payload: { pull_request: current } }; + const context = { repo: { owner: 'NVIDIA', repo: 'TensorRT-LLM' }, payload: options.payload || { pull_request: current } }; await execute(github, context, { info: () => {} }, { env: options.env || {} }, class extends Date { static now() { return now; } }); return { writes, calls }; } test('first five pass; sixth receives explanation before closure', async () => { - for (let n = 1; n <= 5; n++) assert.deepEqual((await run({ current: pr(n) })).writes, []); + for (let n = 1; n <= 5; n++) assert.deepEqual((await run({ current: pr(n), history: Array.from({ length: n }, (_, i) => pr(i + 1)) })).writes, []); const { writes } = await run(); assert.deepEqual(writes.map((w) => w.kind), ['comment', 'close']); assert.match(writes[0].body, /submitted 6 PRs/); @@ -87,16 +91,17 @@ test('counts drafts and closed unmerged PRs, but not ordinary issues or another history.push(pr(0, { pull_request: undefined }), pr(-1, { user: { id: 99 } })); assert.equal((await run({ history })).writes.length, 2); }); -test('exact rolling boundary excluded; later PRs cannot penalize earlier submissions', async () => { +test('submission window excludes its lower boundary and later PRs', async () => { const current = pr(6); const boundary = new Date(Date.parse(current.created_at) - 24 * hour).toISOString(); - const history = [pr(1, { created_at: boundary }), ...[2, 3, 4, 5].map((n) => pr(n)), pr(7)]; + const history = [pr(1, { created_at: boundary }), ...[2, 3, 4, 5].map((n) => pr(n)), pr(7)] + .map((item) => ({ ...item, state: 'closed' })); assert.deepEqual((await run({ current, history })).writes, []); }); -test('same-second bursts ordered by PR number, regardless of execution order', async () => { - const history = Array.from({ length: 10 }, (_, i) => pr(i + 1, { created_at: pr(1).created_at })); - assert.deepEqual((await run({ current: history[4], history })).writes, []); - assert.equal((await run({ current: history[5], history })).writes.length, 2); +test('submission quota orders same-second bursts by PR number', async () => { + const history = Array.from({ length: 10 }, (_, i) => pr(i + 1, { created_at: pr(1).created_at, state: 'closed' })); + assert.deepEqual((await run({ current: { ...history[4], state: 'open' }, history })).writes, []); + assert.equal((await run({ current: { ...history[5], state: 'open' }, history })).writes.length, 2); }); test('history pagination and duplicate entries do not change quota', async () => { const first = Array.from({ length: 100 }, () => pr(1)); @@ -124,19 +129,19 @@ test('dry run, closed PRs, and exemptions added during evaluation make no writes assert.deepEqual((await run({ recheck: pr(6, { labels: [{ name: 'pr-rate-limit-exempt' }] }) })).writes, []); assert.deepEqual((await run({ recheck: pr(6, { state: 'closed' }) })).writes, []); }); -test('reopening after cooldown is allowed; reopening before it is still limited', async () => { - assert.deepEqual((await run({ current: pr(6, { created_at: new Date(now - 24 * hour).toISOString() }) })).writes, []); +test('reopening after cooldown requires capacity; submission cooldown still applies before it', async () => { + assert.deepEqual((await run({ current: pr(6, { created_at: new Date(now - 24 * hour).toISOString() }), history: [] })).writes, []); assert.equal((await run()).writes.length, 2); }); test('reruns reuse only bot-authored comments and retry closure', async () => { - const body = ''; + const body = ''; const trusted = { user: { login: 'github-actions[bot]' }, body }; assert.deepEqual((await run({ comments: [trusted] })).writes.map((w) => w.kind), ['close']); assert.equal((await run({ comments: [{ user: { login: 'new-user' }, body }] })).writes.length, 2); }); test('API failures, incomplete history and invalid timestamps abort without moderation', async () => { for (const options of [{ readError: true }, { historyError: true }, { commentError: true }, - { permissionError: true }, { commentsError: true }, + { permissionError: true }, { commentsError: true }, { openHistoryError: true }, { pages: [Array.from({ length: 100 }, () => pr(1))], failedPage: 2 }, { history: [pr(1, { created_at: 'invalid' })] }, { current: pr(6, { created_at: 'invalid' }) }, @@ -158,6 +163,63 @@ test('workflow uses trusted inline code, minimal permissions and per-PR concurre assert.match(workflow, /types: \[opened, reopened, ready_for_review\]/); assert.match(workflow, /pull-requests: write/); assert.match(workflow, /issues: read/); - assert.match(workflow, /group: pr-rate-limit-\$\{\{ github.event.pull_request.number \}\}/); + assert.match(workflow, /group: pr-rate-limit-\$\{\{ github.event.pull_request.number \|\| inputs.pr_number \}\}/); assert.doesNotMatch(workflow, /actions\/checkout|head.sha|secrets\./); }); + +test('manual recovery evaluates the requested PR and rejects invalid input before API calls', async () => { + const { writes, calls } = await run({ payload: { inputs: { pr_number: '6' } } }); + assert.deepEqual(writes.map((w) => w.kind), ['comment', 'close']); + assert.ok(calls.every((number) => number === 6)); + for (const pr_number of ['', '0', '-1', '1.5', '06', ' 6', '6x', '9007199254740992']) { + await assert.rejects(run({ payload: { inputs: { pr_number } }, readError: true }), /positive integer PR number/); + } + await assert.rejects(run({ payload: {}, readError: true }), /positive integer PR number/); +}); + +test('manual recovery preserves dry run, exemptions, cooldown and comment reconciliation', async () => { + const payload = { inputs: { pr_number: '6' } }; + assert.deepEqual((await run({ payload, env: { DRY_RUN: 'true' } })).writes, []); + assert.deepEqual((await run({ payload, permission: 'write' })).writes, []); + assert.deepEqual((await run({ payload, current: pr(6, { created_at: new Date(now - 24 * hour).toISOString() }), history: [] })).writes, []); + const comments = [{ user: { login: 'github-actions[bot]' }, body: '' }]; + assert.deepEqual((await run({ payload, comments })).writes.map((w) => w.kind), ['close']); +}); + +test('privileged action is pinned and manual runs are restricted to the default branch', () => { + assert.match(workflow, /uses: actions\/github-script@[0-9a-f]{40} # v8/); + assert.match(workflow, /workflow_dispatch:/); + assert.match(workflow, /github.event_name != 'workflow_dispatch' \|\|/); + assert.ok(workflow.includes("github.ref == format('refs/heads/{0}', github.event.repository.default_branch)")); +}); + +test('open-PR limit prevents reopening a backlog after the submission cooldown', async () => { + const current = pr(6, { created_at: new Date(now - 48 * hour).toISOString() }); + const history = Array.from({ length: 5 }, (_, i) => pr(i + 1, { draft: true })); + const { writes } = await run({ current, history }); + assert.deepEqual(writes.map((w) => w.kind), ['comment', 'close']); + assert.match(writes[0].body, /currently have 6 open PRs/); + assert.match(writes[0].body, /fewer than 5 of your other PRs are open/); + assert.doesNotMatch(writes[0].body, /cooldown ends/); + assert.deepEqual((await run({ current, history: history.slice(0, 4) })).writes, []); +}); + +test('old reopened PR is allowed when capacity becomes available during evaluation', async () => { + const current = pr(6, { created_at: new Date(now - 48 * hour).toISOString() }); + assert.deepEqual((await run({ current, openHistory: [pr(1), pr(2)] })).writes, []); +}); + +test('open-PR limit applies even when recent submissions are below five', async () => { + const history = Array.from({ length: 5 }, (_, i) => pr(i + 1, { created_at: new Date(now - 48 * hour).toISOString() })); + const { writes } = await run({ history }); + assert.match(writes[0].body, /currently have 6 open PRs/); + assert.doesNotMatch(writes[0].body, /cooldown ends/); +}); + +test('a reopened backlog of old PRs stays subject to the open cap during manual recovery', async () => { + const history = Array.from({ length: 100 }, (_, i) => pr(i + 1, { created_at: new Date(now - 48 * hour).toISOString() })); + const current = history[99]; + const { writes } = await run({ current, pages: [history, []], openHistory: history, payload: { inputs: { pr_number: '100' } } }); + assert.match(writes[0].body, /currently have 100 open PRs/); + assert.equal(writes[1].pull_number, 100); +}); diff --git a/.github/workflows/pr-rate-limit.yml b/.github/workflows/pr-rate-limit.yml index 42d93da1f64b..de8030152de8 100644 --- a/.github/workflows/pr-rate-limit.yml +++ b/.github/workflows/pr-rate-limit.yml @@ -18,6 +18,12 @@ name: New Contributor PR Rate Limit on: pull_request_target: types: [opened, reopened, ready_for_review] + workflow_dispatch: + inputs: + pr_number: + description: PR number to reevaluate after a failed moderation run + required: true + type: string permissions: contents: read @@ -27,30 +33,38 @@ permissions: # Serialize retries for one PR, not all submissions by an author: a shared # author group can discard pending runs during the very burst we must handle. concurrency: - group: pr-rate-limit-${{ github.event.pull_request.number }} + group: pr-rate-limit-${{ github.event.pull_request.number || inputs.pr_number }} cancel-in-progress: false jobs: rate-limit: - if: github.repository == 'NVIDIA/TensorRT-LLM' + if: >- + github.repository == 'NVIDIA/TensorRT-LLM' && + (github.event_name != 'workflow_dispatch' || + github.ref == format('refs/heads/{0}', github.event.repository.default_branch)) runs-on: ubuntu-latest timeout-minutes: 10 steps: # No checkout or execution of PR code in this privileged workflow. # Administrators can set PR_RATE_LIMIT_DRY_RUN=true to disable writes. # PR_RATE_LIMIT_EXEMPT_USERS is a comma-separated list of trusted logins. - - uses: actions/github-script@v8 + - uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 env: DRY_RUN: ${{ vars.PR_RATE_LIMIT_DRY_RUN }} EXEMPT_USERS: ${{ vars.PR_RATE_LIMIT_EXEMPT_USERS }} with: script: | - const LIMIT = 5; + const SUBMISSION_LIMIT = 5; + const OPEN_LIMIT = 5; const WINDOW_MS = 24 * 60 * 60 * 1000; const EXEMPT_LABEL = 'pr-rate-limit-exempt'; - const MARKER = ''; + const MARKER = ''; const { owner, repo } = context.repo; - const pull_number = context.payload.pull_request.number; + const rawNumber = String(context.payload.pull_request?.number ?? context.payload.inputs?.pr_number ?? ''); + const pull_number = Number(rawNumber); + if (!/^[1-9][0-9]*$/.test(rawNumber) || !Number.isSafeInteger(pull_number)) { + throw new Error('A positive integer PR number is required; no moderation performed.'); + } const now = Date.now(); const getPull = async (number) => (await github.rest.pulls.get({ owner, repo, pull_number: number, @@ -60,9 +74,8 @@ jobs: if (!Number.isFinite(created) || created > now) { throw new Error('Invalid pull request creation time; no moderation performed.'); } - // Reopening does not constitute a new submission. Once the original - // submission is 24 hours old, the contributor may reopen it for review. - if (pull.state !== 'open' || now >= created + WINDOW_MS) return; + // The submission cooldown expires, but the open-PR limit never ages out. + if (pull.state !== 'open') return; const login = pull.user.login; const exemptUsers = (process.env.EXEMPT_USERS || '').split(',') .map((value) => value.trim().toLowerCase()).filter(Boolean); @@ -76,19 +89,21 @@ jobs: // The creator-filtered issues API includes PRs in every state, without // relying on the eventually consistent search index. Complete the listing // before making any changes. Excessive history or API failures abort safely. - const history = new Map(); - let complete = false; - for (let page = 1; page <= 20; page += 1) { - const { data } = await github.rest.issues.listForRepo({ - owner, repo, creator: login, state: 'all', sort: 'created', - direction: 'desc', per_page: 100, page, - }); - for (const item of data) { - if (item.pull_request && item.user.id === pull.user.id) history.set(item.number, item); + const readHistory = async (state) => { + const items = new Map(); + for (let page = 1; page <= 20; page += 1) { + const { data } = await github.rest.issues.listForRepo({ + owner, repo, creator: login, state, sort: 'created', + direction: 'desc', per_page: 100, page, + }); + for (const item of data) { + if (item.pull_request && item.user.id === pull.user.id) items.set(item.number, item); + } + if (data.length < 100) return items; } - if (data.length < 100) { complete = true; break; } - } - if (!complete) throw new Error('Contributor history exceeded 2000 items; no moderation performed.'); + throw new Error('Contributor history could not be completed within 20 pages; no moderation performed.'); + }; + const history = await readHistory('all'); // Include the triggering PR even if its listing has not caught up yet. history.set(pull.number, pull); const preceding = [...history.values()].filter((item) => { @@ -97,7 +112,9 @@ jobs: return time > created - WINDOW_MS && (time < created || (time === created && item.number <= pull.number)); }); - if (preceding.length <= LIMIT) return; + const exceedsSubmissions = () => preceding.length > SUBMISSION_LIMIT && Date.now() < created + WINDOW_MS; + let openCount = [...history.values()].filter((item) => item.state === 'open').length; + if (!exceedsSubmissions() && openCount <= OPEN_LIMIT) return; // A closed PR is not necessarily a contribution: require an actual merge. // Check history directly rather than trusting author_association or a // search result that may not yet include a recent merge. @@ -107,25 +124,38 @@ jobs: if (previous.merged_at) return; } } + const comments = await github.paginate(github.rest.issues.listComments, { + owner, repo, issue_number: pull_number, per_page: 100, + }); + // Refresh capacity before writing: another PR may have closed meanwhile. + const openHistory = await readHistory('open'); + const current = await getPull(pull_number); + if (current.state !== 'open' || hasExemptLabel(current)) return; + openHistory.set(current.number, current); + openCount = [...openHistory.values()].filter((item) => item.state === 'open').length; + const submissionExceeded = exceedsSubmissions(); + if (!submissionExceeded && openCount <= OPEN_LIMIT) return; + const reasons = []; + if (submissionExceeded) reasons.push('you submitted ' + preceding.length + + ' PRs in the 24-hour window ending with this submission (limit: ' + SUBMISSION_LIMIT + ')'); + if (openCount > OPEN_LIMIT) reasons.push('you currently have ' + openCount + + ' open PRs, including this one (limit: ' + OPEN_LIMIT + ')'); const body = [ - 'You submitted ' + preceding.length + ' PRs in the 24-hour window ending with this submission. ' + - 'Contributors with no merged PRs in this repository are limited to ' + LIMIT + ' submissions per rolling 24 hours, including drafts and closed PRs.', - 'Closing this PR because it exceeds that limit. Please consolidate related changes into coherent, validated PRs. ' + - 'You may reopen this PR after ' + new Date(created + WINDOW_MS).toISOString() + - ', or ask a maintainer for an exception.', + 'Closing this PR because ' + reasons.join('; ') + '.', + 'Contributors with no merged PRs in this repository may submit up to ' + SUBMISSION_LIMIT + + ' PRs per rolling 24 hours and keep up to ' + OPEN_LIMIT + ' PRs open, including drafts. ' + + 'Closed PRs still count toward the submission limit.', + (submissionExceeded ? 'The submission cooldown ends at ' + new Date(created + WINDOW_MS).toISOString() + '. ' : '') + + 'You may reopen this PR only when fewer than ' + OPEN_LIMIT + ' of your other PRs are open' + + (submissionExceeded ? ' and the submission cooldown has ended' : '') + '. ' + + 'Please consolidate related changes into coherent, validated PRs, or ask a maintainer for an exception.', MARKER, ].join('\n\n'); - core.info('PR #' + pull_number + ': ' + preceding.length + ' submissions; limit ' + LIMIT); + core.info('PR #' + pull_number + ': ' + reasons.join('; ')); if (process.env.DRY_RUN === 'true') { core.info('Dry run: would comment and close.'); return; } - const comments = await github.paginate(github.rest.issues.listComments, { - owner, repo, issue_number: pull_number, per_page: 100, - }); - // Recheck after pagination, immediately before writing. - const current = await getPull(pull_number); - if (current.state !== 'open' || hasExemptLabel(current) || Date.now() >= created + WINDOW_MS) return; if (!comments.some((comment) => comment.user?.login === 'github-actions[bot]' && comment.body?.includes(MARKER))) { // Do not blindly retry a comment write: a timeout may follow a successful // write. A rerun reconciles against the bot-authored marker above. diff --git a/AGENTS.md b/AGENTS.md index eb06ee73213f..dff1b82fa9b2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -168,7 +168,7 @@ See [CI overview](docs/source/developer-guide/ci-overview.md) for full details. ### Triggering CI The new-contributor submission limit is implemented in `.github/workflows/pr-rate-limit.yml`; -see `CONTRIBUTING.md` for the five-PR/24-hour policy and maintainer exemptions. Its mocked API tests +see `CONTRIBUTING.md` for the five-PR/24-hour submission limit, five-open-PR cap, exemptions and manual recovery. Its mocked API tests run with `node --test .github/scripts/pr_rate_limit.test.js` and are registered in Release Checks. CI is triggered by posting comments on the PR. Basic commands: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index bc023e051de3..e87e6af6cc31 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -120,10 +120,13 @@ Meanwhile, please add the "release blocker" label to any PRs that could potentia ### Submission rate for new contributors -Contributors with no merged PRs in this repository may submit up to five PRs per rolling 24 hours. -Drafts and closed PRs count toward this limit. Please consolidate related changes into coherent, -validated PRs. Excess submissions receive an explanation and are automatically closed after creation. -The comment gives a time, 24 hours after that PR's creation, when it may be reopened for review. +Contributors with no merged PRs in this repository may submit up to five PRs per rolling 24 hours +and keep up to five PRs open at a time. Drafts count toward both limits; closed PRs still count +as submissions. Please consolidate related changes into coherent, validated PRs. Excess submissions +or reopenings are automatically closed after creation or reopening. The initial explanation includes +the observed counts; later retries reuse that comment, so its counts describe the original evaluation. An excess submission has a cooldown of 24 hours from its creation. After that cooldown, +reopening still requires fewer than five other open PRs; waiting does not exempt an open backlog. +Concurrent submissions or reopenings may be closed while the observed open count exceeds five. Users with write access and bot accounts are exempt. Maintainers can apply `pr-rate-limit-exempt` to an individual PR before reopening it. Administrators can exempt trusted contributors through the @@ -131,6 +134,13 @@ comma-separated `PR_RATE_LIMIT_EXEMPT_USERS` repository variable, or set `PR_RAT to log decisions without commenting or closing. The workflow does not prevent independently triggered CI jobs from starting. If GitHub history cannot be read completely, it fails without closing the PR. +Maintainers should monitor failed **New Contributor PR Rate Limit** runs in the Actions tab and retry +after resolving the reported API or history error. Re-run the failed job, or choose **Run workflow** on +the default branch and enter the affected PR number. Manual recovery uses the same exemptions, dry-run +setting and limits, and reuses an existing bot comment before retrying closure. The open-PR limit still +applies after 24 hours. Recovery is manual; there is no scheduled sweep or guarantee of an instantaneous +cap while jobs are pending or GitHub is unavailable. + ### Inactive pull requests TensorRT LLM automatically reviews inactive pull requests each day. This policy applies to both draft and From cf19366a79c203f32ec354d115438ccf62a1ece5 Mon Sep 17 00:00:00 2001 From: Kanghwan Jang <861393+karljang@users.noreply.github.com> Date: Wed, 9 Sep 2026 13:01:55 -0700 Subject: [PATCH 3/6] Clarify maintainer-assisted reopening after moderation Signed-off-by: Kanghwan Jang <861393+karljang@users.noreply.github.com> --- .github/scripts/pr_rate_limit.test.js | 2 +- .github/workflows/pr-rate-limit.yml | 2 +- CONTRIBUTING.md | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/scripts/pr_rate_limit.test.js b/.github/scripts/pr_rate_limit.test.js index 93f04ad0abab..e511723682fb 100644 --- a/.github/scripts/pr_rate_limit.test.js +++ b/.github/scripts/pr_rate_limit.test.js @@ -199,7 +199,7 @@ test('open-PR limit prevents reopening a backlog after the submission cooldown', const { writes } = await run({ current, history }); assert.deepEqual(writes.map((w) => w.kind), ['comment', 'close']); assert.match(writes[0].body, /currently have 6 open PRs/); - assert.match(writes[0].body, /fewer than 5 of your other PRs are open/); + assert.match(writes[0].body, /ask a maintainer to reopen this PR when fewer than 5 of your other PRs are open/); assert.doesNotMatch(writes[0].body, /cooldown ends/); assert.deepEqual((await run({ current, history: history.slice(0, 4) })).writes, []); }); diff --git a/.github/workflows/pr-rate-limit.yml b/.github/workflows/pr-rate-limit.yml index de8030152de8..7fa55978b9fd 100644 --- a/.github/workflows/pr-rate-limit.yml +++ b/.github/workflows/pr-rate-limit.yml @@ -146,7 +146,7 @@ jobs: ' PRs per rolling 24 hours and keep up to ' + OPEN_LIMIT + ' PRs open, including drafts. ' + 'Closed PRs still count toward the submission limit.', (submissionExceeded ? 'The submission cooldown ends at ' + new Date(created + WINDOW_MS).toISOString() + '. ' : '') + - 'You may reopen this PR only when fewer than ' + OPEN_LIMIT + ' of your other PRs are open' + + 'You may ask a maintainer to reopen this PR when fewer than ' + OPEN_LIMIT + ' of your other PRs are open' + (submissionExceeded ? ' and the submission cooldown has ended' : '') + '. ' + 'Please consolidate related changes into coherent, validated PRs, or ask a maintainer for an exception.', MARKER, diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e87e6af6cc31..8934d59d309a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -125,7 +125,8 @@ and keep up to five PRs open at a time. Drafts count toward both limits; closed as submissions. Please consolidate related changes into coherent, validated PRs. Excess submissions or reopenings are automatically closed after creation or reopening. The initial explanation includes the observed counts; later retries reuse that comment, so its counts describe the original evaluation. An excess submission has a cooldown of 24 hours from its creation. After that cooldown, -reopening still requires fewer than five other open PRs; waiting does not exempt an open backlog. +ask a maintainer to reopen the PR when fewer than five other PRs are open; waiting does not exempt an +open backlog. GitHub may prevent authors from reopening PRs closed by the bot. Concurrent submissions or reopenings may be closed while the observed open count exceeds five. Users with write access and bot accounts are exempt. Maintainers can apply `pr-rate-limit-exempt` From f19910000f6e2916cec3bf0adc8872aae95f670f Mon Sep 17 00:00:00 2001 From: Kanghwan Jang <861393+karljang@users.noreply.github.com> Date: Wed, 9 Sep 2026 13:35:09 -0700 Subject: [PATCH 4/6] Use an open-PR cap and address moderation review feedback Signed-off-by: Kanghwan Jang <861393+karljang@users.noreply.github.com> --- .github/scripts/pr_rate_limit.test.js | 99 +++++++++++++++++---------- .github/workflows/pr-rate-limit.yml | 76 +++++++++----------- AGENTS.md | 4 +- CONTRIBUTING.md | 40 ++++++----- 4 files changed, 119 insertions(+), 100 deletions(-) diff --git a/.github/scripts/pr_rate_limit.test.js b/.github/scripts/pr_rate_limit.test.js index e511723682fb..c8e18b38cd14 100644 --- a/.github/scripts/pr_rate_limit.test.js +++ b/.github/scripts/pr_rate_limit.test.js @@ -16,8 +16,15 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const { test } = require('node:test'); const workflow = fs.readFileSync(`${__dirname}/../workflows/pr-rate-limit.yml`, 'utf8'); -const script = workflow.split(' script: |\n')[1] - .split('\n').map((line) => line.replace(/^ {12}/, '')).join('\n'); +function extractScript(source) { + const parts = source.split(' script: |\n'); + assert.equal(parts.length, 2, 'Expected exactly one inline script block'); + const lines = parts[1].split('\n'); + const end = lines.findIndex((line) => line.trim() && !line.startsWith(' ')); + return lines.slice(0, end < 0 ? lines.length : end) + .map((line) => line.replace(/^ {12}/, '')).join('\n'); +} +const script = extractScript(workflow); const execute = new (Object.getPrototypeOf(async function () {}).constructor)( 'github', 'context', 'core', 'process', 'Date', script, ); @@ -32,6 +39,7 @@ async function run(options = {}) { const current = options.current || pr(6); const writes = options.writes || []; const calls = []; + const warnings = []; const github = { rest: { pulls: { get: async ({ pull_number }) => { @@ -47,7 +55,7 @@ async function run(options = {}) { }, }, repos: { getCollaboratorPermissionLevel: async () => { - if (options.permissionError) throw new Error('permission failed'); + if (options.permissionError) throw Object.assign(new Error('permission failed'), { status: options.permissionStatus }); return { data: { permission: options.permission || 'read', user: { permissions: { push: options.push } } } }; } }, issues: { @@ -73,46 +81,39 @@ async function run(options = {}) { return options.comments || []; } }; const context = { repo: { owner: 'NVIDIA', repo: 'TensorRT-LLM' }, payload: options.payload || { pull_request: current } }; - await execute(github, context, { info: () => {} }, { env: options.env || {} }, + await execute(github, context, { info: () => {}, warning: (message) => warnings.push(message) }, { env: options.env || {} }, class extends Date { static now() { return now; } }); - return { writes, calls }; + return { writes, calls, warnings }; } test('first five pass; sixth receives explanation before closure', async () => { for (let n = 1; n <= 5; n++) assert.deepEqual((await run({ current: pr(n), history: Array.from({ length: n }, (_, i) => pr(i + 1)) })).writes, []); const { writes } = await run(); assert.deepEqual(writes.map((w) => w.kind), ['comment', 'close']); - assert.match(writes[0].body, /submitted 6 PRs/); + assert.match(writes[0].body, /currently have 6 open PRs/); assert.match(writes[0].body, /no merged PRs/); - assert.match(writes[0].body, /2026-09-10T11:00:06.000Z/); + assert.match(writes[0].body, /wait until your existing PRs are reviewed or merged/); + assert.doesNotMatch(writes[0].body, /24|cooldown|consolidate|rolling/); assert.equal(writes[1].state, 'closed'); }); -test('counts drafts and closed unmerged PRs, but not ordinary issues or another author', async () => { - const history = Array.from({ length: 5 }, (_, i) => pr(i + 1, { draft: true, state: 'closed' })); - history.push(pr(0, { pull_request: undefined }), pr(-1, { user: { id: 99 } })); +test('counts drafts but excludes closed PRs, ordinary issues and another author', async () => { + const history = Array.from({ length: 5 }, (_, i) => pr(i + 1, { draft: true })); + history.push(pr(-2, { state: 'closed' }), pr(0, { pull_request: undefined }), pr(-1, { user: { id: 99 } })); assert.equal((await run({ history })).writes.length, 2); }); -test('submission window excludes its lower boundary and later PRs', async () => { - const current = pr(6); - const boundary = new Date(Date.parse(current.created_at) - 24 * hour).toISOString(); - const history = [pr(1, { created_at: boundary }), ...[2, 3, 4, 5].map((n) => pr(n)), pr(7)] - .map((item) => ({ ...item, state: 'closed' })); - assert.deepEqual((await run({ current, history })).writes, []); -}); -test('submission quota orders same-second bursts by PR number', async () => { - const history = Array.from({ length: 10 }, (_, i) => pr(i + 1, { created_at: pr(1).created_at, state: 'closed' })); - assert.deepEqual((await run({ current: { ...history[4], state: 'open' }, history })).writes, []); - assert.equal((await run({ current: { ...history[5], state: 'open' }, history })).writes.length, 2); -}); + + test('history pagination and duplicate entries do not change quota', async () => { const first = Array.from({ length: 100 }, () => pr(1)); const { writes } = await run({ pages: [first, [pr(2), pr(3), pr(4), pr(5)]] }); assert.equal(writes.length, 2); - assert.match(writes[0].body, /submitted 6 PRs/); + assert.match(writes[0].body, /currently have 6 open PRs/); }); test('actual merged history exempts; author association alone does not', async () => { const history = Array.from({ length: 6 }, (_, i) => pr(i + 1)); - history.push(pr(0, { state: 'closed' })); - assert.deepEqual((await run({ history, merged: true })).writes, []); + history.push(pr(0, { state: 'closed', pull_request: { merged_at: '2026-09-08T00:00:00Z' } })); + const result = await run({ history }); + assert.deepEqual(result.writes, []); + assert.deepEqual(result.calls, [6]); assert.equal((await run({ current: pr(6, { author_association: 'CONTRIBUTOR' }) })).writes.length, 2); }); test('maintainers, bots, allowlisted users and labeled PRs are exempt', async () => { @@ -129,23 +130,17 @@ test('dry run, closed PRs, and exemptions added during evaluation make no writes assert.deepEqual((await run({ recheck: pr(6, { labels: [{ name: 'pr-rate-limit-exempt' }] }) })).writes, []); assert.deepEqual((await run({ recheck: pr(6, { state: 'closed' }) })).writes, []); }); -test('reopening after cooldown requires capacity; submission cooldown still applies before it', async () => { - assert.deepEqual((await run({ current: pr(6, { created_at: new Date(now - 24 * hour).toISOString() }), history: [] })).writes, []); - assert.equal((await run()).writes.length, 2); -}); + test('reruns reuse only bot-authored comments and retry closure', async () => { const body = ''; const trusted = { user: { login: 'github-actions[bot]' }, body }; assert.deepEqual((await run({ comments: [trusted] })).writes.map((w) => w.kind), ['close']); assert.equal((await run({ comments: [{ user: { login: 'new-user' }, body }] })).writes.length, 2); }); -test('API failures, incomplete history and invalid timestamps abort without moderation', async () => { +test('unexpected API failures abort without moderation', async () => { for (const options of [{ readError: true }, { historyError: true }, { commentError: true }, { permissionError: true }, { commentsError: true }, { openHistoryError: true }, - { pages: [Array.from({ length: 100 }, () => pr(1))], failedPage: 2 }, - { history: [pr(1, { created_at: 'invalid' })] }, - { current: pr(6, { created_at: 'invalid' }) }, - { pages: Array.from({ length: 20 }, () => Array.from({ length: 100 }, () => pr(1))) }]) { + { pages: [Array.from({ length: 100 }, () => pr(1))], failedPage: 2 }]) { const writes = []; await assert.rejects(run({ ...options, writes })); assert.deepEqual(writes, []); @@ -177,7 +172,7 @@ test('manual recovery evaluates the requested PR and rejects invalid input befor await assert.rejects(run({ payload: {}, readError: true }), /positive integer PR number/); }); -test('manual recovery preserves dry run, exemptions, cooldown and comment reconciliation', async () => { +test('manual recovery preserves dry run, exemptions, capacity and comment reconciliation', async () => { const payload = { inputs: { pr_number: '6' } }; assert.deepEqual((await run({ payload, env: { DRY_RUN: 'true' } })).writes, []); assert.deepEqual((await run({ payload, permission: 'write' })).writes, []); @@ -193,7 +188,7 @@ test('privileged action is pinned and manual runs are restricted to the default assert.ok(workflow.includes("github.ref == format('refs/heads/{0}', github.event.repository.default_branch)")); }); -test('open-PR limit prevents reopening a backlog after the submission cooldown', async () => { +test('open-PR limit prevents reopening an old backlog', async () => { const current = pr(6, { created_at: new Date(now - 48 * hour).toISOString() }); const history = Array.from({ length: 5 }, (_, i) => pr(i + 1, { draft: true })); const { writes } = await run({ current, history }); @@ -223,3 +218,35 @@ test('a reopened backlog of old PRs stays subject to the open cap during manual assert.match(writes[0].body, /currently have 100 open PRs/); assert.equal(writes[1].pull_number, 100); }); + +test('closed submissions never consume capacity, regardless of timestamps', async () => { + const history = Array.from({ length: 30 }, (_, i) => pr(i + 1, { state: 'closed', created_at: 'invalid' })); + assert.deepEqual((await run({ history })).writes, []); +}); +test('permission denials and incomplete history warn and skip', async () => { + for (const options of [ + { permissionError: true, permissionStatus: 403 }, + { permissionError: true, permissionStatus: 404 }, + { pages: Array.from({ length: 20 }, () => Array.from({ length: 100 }, () => pr(1))) }, + ]) { + const result = await run(options); + assert.deepEqual(result.writes, []); + assert.equal(result.warnings.length, 1); + } +}); +test('configured open cap is enforced and invalid configuration skips all API calls', async () => { + assert.deepEqual((await run({ env: { MAX_OPEN: '6' } })).writes, []); + const { writes } = await run({ env: { MAX_OPEN: '3' } }); + assert.match(writes[0].body, /limit: 3/); + for (const value of ['0', '-1', '1.5', 'bad', 'Infinity', '9007199254740992']) { + const result = await run({ env: { MAX_OPEN: value } }); + assert.deepEqual(result.calls, []); + assert.deepEqual(result.writes, []); + assert.equal(result.warnings.length, 1); + } +}); +test('script extraction stops at a subsequent step and rejects ambiguous blocks', () => { + assert.equal(extractScript(workflow + '\n - run: echo later\n'), script); + assert.throws(() => extractScript(''), /exactly one/); + assert.throws(() => extractScript(workflow + ' script: |\n'), /exactly one/); +}); diff --git a/.github/workflows/pr-rate-limit.yml b/.github/workflows/pr-rate-limit.yml index 7fa55978b9fd..9f949c5d3cc0 100644 --- a/.github/workflows/pr-rate-limit.yml +++ b/.github/workflows/pr-rate-limit.yml @@ -52,11 +52,14 @@ jobs: env: DRY_RUN: ${{ vars.PR_RATE_LIMIT_DRY_RUN }} EXEMPT_USERS: ${{ vars.PR_RATE_LIMIT_EXEMPT_USERS }} + MAX_OPEN: ${{ vars.PR_RATE_LIMIT_MAX_OPEN }} with: script: | - const SUBMISSION_LIMIT = 5; - const OPEN_LIMIT = 5; - const WINDOW_MS = 24 * 60 * 60 * 1000; + const OPEN_LIMIT = Number(process.env.MAX_OPEN?.trim() || '5'); + if (!Number.isSafeInteger(OPEN_LIMIT) || OPEN_LIMIT < 1) { + core.warning('Invalid PR_RATE_LIMIT_MAX_OPEN; no moderation performed.'); + return; + } const EXEMPT_LABEL = 'pr-rate-limit-exempt'; const MARKER = ''; const { owner, repo } = context.repo; @@ -65,25 +68,27 @@ jobs: if (!/^[1-9][0-9]*$/.test(rawNumber) || !Number.isSafeInteger(pull_number)) { throw new Error('A positive integer PR number is required; no moderation performed.'); } - const now = Date.now(); const getPull = async (number) => (await github.rest.pulls.get({ owner, repo, pull_number: number, })).data; const pull = await getPull(pull_number); - const created = Date.parse(pull.created_at); - if (!Number.isFinite(created) || created > now) { - throw new Error('Invalid pull request creation time; no moderation performed.'); - } - // The submission cooldown expires, but the open-PR limit never ages out. + // The open-PR cap applies regardless of creation time or reopening age. if (pull.state !== 'open') return; const login = pull.user.login; const exemptUsers = (process.env.EXEMPT_USERS || '').split(',') .map((value) => value.trim().toLowerCase()).filter(Boolean); const hasExemptLabel = (pr) => pr.labels.some((label) => label.name === EXEMPT_LABEL); if (pull.user.type === 'Bot' || exemptUsers.includes(login.toLowerCase()) || hasExemptLabel(pull)) return; - const { data: permission } = await github.rest.repos.getCollaboratorPermissionLevel({ - owner, repo, username: login, - }); + let permission; + try { + ({ data: permission } = await github.rest.repos.getCollaboratorPermissionLevel({ + owner, repo, username: login, + })); + } catch (error) { + if (![403, 404].includes(error.status)) throw error; + core.warning('Cannot determine contributor permissions (' + error.status + '); no moderation performed.'); + return; + } if (permission.user?.permissions?.push || ['admin', 'write', 'maintain'].includes(permission.permission)) return; // The creator-filtered issues API includes PRs in every state, without @@ -101,57 +106,42 @@ jobs: } if (data.length < 100) return items; } - throw new Error('Contributor history could not be completed within 20 pages; no moderation performed.'); + core.warning('Contributor history could not be completed within 20 pages; no moderation performed.'); + return null; }; const history = await readHistory('all'); + if (!history) return; // Include the triggering PR even if its listing has not caught up yet. history.set(pull.number, pull); - const preceding = [...history.values()].filter((item) => { - const time = Date.parse(item.created_at); - if (!Number.isFinite(time)) throw new Error('Invalid history timestamp; no moderation performed.'); - return time > created - WINDOW_MS && - (time < created || (time === created && item.number <= pull.number)); - }); - const exceedsSubmissions = () => preceding.length > SUBMISSION_LIMIT && Date.now() < created + WINDOW_MS; let openCount = [...history.values()].filter((item) => item.state === 'open').length; - if (!exceedsSubmissions() && openCount <= OPEN_LIMIT) return; + if (openCount <= OPEN_LIMIT) return; // A closed PR is not necessarily a contribution: require an actual merge. // Check history directly rather than trusting author_association or a // search result that may not yet include a recent merge. - for (const item of history.values()) { - if (item.state === 'closed') { - const previous = await getPull(item.number); - if (previous.merged_at) return; - } - } + if ([...history.values()].some((item) => item.pull_request?.merged_at)) return; const comments = await github.paginate(github.rest.issues.listComments, { owner, repo, issue_number: pull_number, per_page: 100, }); // Refresh capacity before writing: another PR may have closed meanwhile. const openHistory = await readHistory('open'); + if (!openHistory) return; const current = await getPull(pull_number); if (current.state !== 'open' || hasExemptLabel(current)) return; openHistory.set(current.number, current); openCount = [...openHistory.values()].filter((item) => item.state === 'open').length; - const submissionExceeded = exceedsSubmissions(); - if (!submissionExceeded && openCount <= OPEN_LIMIT) return; - const reasons = []; - if (submissionExceeded) reasons.push('you submitted ' + preceding.length + - ' PRs in the 24-hour window ending with this submission (limit: ' + SUBMISSION_LIMIT + ')'); - if (openCount > OPEN_LIMIT) reasons.push('you currently have ' + openCount + - ' open PRs, including this one (limit: ' + OPEN_LIMIT + ')'); + if (openCount <= OPEN_LIMIT) return; + const reason = 'you currently have ' + openCount + + ' open PRs, including this one (limit: ' + OPEN_LIMIT + ')'; const body = [ - 'Closing this PR because ' + reasons.join('; ') + '.', - 'Contributors with no merged PRs in this repository may submit up to ' + SUBMISSION_LIMIT + - ' PRs per rolling 24 hours and keep up to ' + OPEN_LIMIT + ' PRs open, including drafts. ' + - 'Closed PRs still count toward the submission limit.', - (submissionExceeded ? 'The submission cooldown ends at ' + new Date(created + WINDOW_MS).toISOString() + '. ' : '') + - 'You may ask a maintainer to reopen this PR when fewer than ' + OPEN_LIMIT + ' of your other PRs are open' + - (submissionExceeded ? ' and the submission cooldown has ended' : '') + '. ' + - 'Please consolidate related changes into coherent, validated PRs, or ask a maintainer for an exception.', + 'Closing this PR because ' + reason + '.', + 'Contributors with no merged PRs in this repository may keep up to ' + OPEN_LIMIT + ' PRs open, including drafts.', + 'Please wait until your existing PRs are reviewed or merged before opening more. ' + + 'Reviewed PRs still count while open. ' + + 'You may ask a maintainer to reopen this PR when fewer than ' + OPEN_LIMIT + ' of your other PRs are open, ' + + 'or ask a maintainer for an exception.', MARKER, ].join('\n\n'); - core.info('PR #' + pull_number + ': ' + reasons.join('; ')); + core.info('PR #' + pull_number + ': ' + reason); if (process.env.DRY_RUN === 'true') { core.info('Dry run: would comment and close.'); return; diff --git a/AGENTS.md b/AGENTS.md index dff1b82fa9b2..2861a1dcf093 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -167,8 +167,8 @@ See [CI overview](docs/source/developer-guide/ci-overview.md) for full details. ### Triggering CI -The new-contributor submission limit is implemented in `.github/workflows/pr-rate-limit.yml`; -see `CONTRIBUTING.md` for the five-PR/24-hour submission limit, five-open-PR cap, exemptions and manual recovery. Its mocked API tests +The new-contributor open-PR cap is implemented in `.github/workflows/pr-rate-limit.yml`; +see `CONTRIBUTING.md` for the default five-open-PR cap, exemptions and manual recovery. Its mocked API tests run with `node --test .github/scripts/pr_rate_limit.test.js` and are registered in Release Checks. CI is triggered by posting comments on the PR. Basic commands: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8934d59d309a..c9a0552ca387 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -118,29 +118,31 @@ For NVIDIA developers, please submit feature or bug fixes to the dedicated bran Meanwhile, please add the "release blocker" label to any PRs that could potentially cause a release delay. -### Submission rate for new contributors - -Contributors with no merged PRs in this repository may submit up to five PRs per rolling 24 hours -and keep up to five PRs open at a time. Drafts count toward both limits; closed PRs still count -as submissions. Please consolidate related changes into coherent, validated PRs. Excess submissions -or reopenings are automatically closed after creation or reopening. The initial explanation includes -the observed counts; later retries reuse that comment, so its counts describe the original evaluation. An excess submission has a cooldown of 24 hours from its creation. After that cooldown, -ask a maintainer to reopen the PR when fewer than five other PRs are open; waiting does not exempt an -open backlog. GitHub may prevent authors from reopening PRs closed by the bot. -Concurrent submissions or reopenings may be closed while the observed open count exceeds five. +### Open pull requests for new contributors + +Contributors with no merged PRs in this repository may keep up to five PRs open at a time, +including drafts. Please wait until your existing PRs are reviewed or merged before opening more. +Reviewed PRs still count while open; closing or merging a PR frees capacity. +Excess PRs are automatically closed after creation or reopening. The explanation includes the +observed open count and configured cap; retries reuse that comment, so its counts describe the +original evaluation. Closed PRs do not consume capacity, and there is no submission cooldown. +Ask a maintainer to reopen a PR when fewer than five other PRs are open. GitHub may prevent authors +from reopening PRs closed by the bot. Concurrent submissions or reopenings may be closed while +the observed open count exceeds the cap, regardless of how old the PRs are. Users with write access and bot accounts are exempt. Maintainers can apply `pr-rate-limit-exempt` to an individual PR before reopening it. Administrators can exempt trusted contributors through the -comma-separated `PR_RATE_LIMIT_EXEMPT_USERS` repository variable, or set `PR_RATE_LIMIT_DRY_RUN=true` +comma-separated `PR_RATE_LIMIT_EXEMPT_USERS` repository variable, set `PR_RATE_LIMIT_MAX_OPEN` +to a positive integer to override the default cap of five, or set `PR_RATE_LIMIT_DRY_RUN=true` to log decisions without commenting or closing. The workflow does not prevent independently triggered -CI jobs from starting. If GitHub history cannot be read completely, it fails without closing the PR. - -Maintainers should monitor failed **New Contributor PR Rate Limit** runs in the Actions tab and retry -after resolving the reported API or history error. Re-run the failed job, or choose **Run workflow** on -the default branch and enter the affected PR number. Manual recovery uses the same exemptions, dry-run -setting and limits, and reuses an existing bot comment before retrying closure. The open-PR limit still -applies after 24 hours. Recovery is manual; there is no scheduled sweep or guarantee of an instantaneous -cap while jobs are pending or GitHub is unavailable. +CI jobs from starting. Incomplete history or unavailable permission checks (403/404) produce warnings +and skip moderation. Invalid cap configuration also warns and skips. Other API failures fail the job. + +Maintainers should monitor warnings and failed **New Contributor PR Rate Limit** runs in the Actions +tab. After resolving the reported problem, re-run the job or choose **Run workflow** on the default +branch and enter the affected PR number. Manual recovery uses the same exemptions, dry-run setting +and cap, and reuses an existing bot comment before retrying closure. Recovery is manual; there is no +scheduled sweep or guarantee of an instantaneous cap while jobs are pending or GitHub is unavailable. ### Inactive pull requests From 0f1fb7b2e5461496298f43c9d72afafd9e9a553b Mon Sep 17 00:00:00 2001 From: Kanghwan Jang <861393+karljang@users.noreply.github.com> Date: Wed, 9 Sep 2026 15:26:33 -0700 Subject: [PATCH 5/6] Cap established contributors at ten open PRs Signed-off-by: Kanghwan Jang <861393+karljang@users.noreply.github.com> --- .github/scripts/pr_rate_limit.test.js | 31 ++++++++++++++++++++++++++- .github/workflows/pr-rate-limit.yml | 18 ++++++++++------ AGENTS.md | 4 ++-- CONTRIBUTING.md | 17 +++++++++------ 4 files changed, 53 insertions(+), 17 deletions(-) diff --git a/.github/scripts/pr_rate_limit.test.js b/.github/scripts/pr_rate_limit.test.js index c8e18b38cd14..7cd03dd382a1 100644 --- a/.github/scripts/pr_rate_limit.test.js +++ b/.github/scripts/pr_rate_limit.test.js @@ -108,7 +108,7 @@ test('history pagination and duplicate entries do not change quota', async () => assert.equal(writes.length, 2); assert.match(writes[0].body, /currently have 6 open PRs/); }); -test('actual merged history exempts; author association alone does not', async () => { +test('actual merged history raises the cap; author association alone does not', async () => { const history = Array.from({ length: 6 }, (_, i) => pr(i + 1)); history.push(pr(0, { state: 'closed', pull_request: { merged_at: '2026-09-08T00:00:00Z' } })); const result = await run({ history }); @@ -116,6 +116,35 @@ test('actual merged history exempts; author association alone does not', async ( assert.deepEqual(result.calls, [6]); assert.equal((await run({ current: pr(6, { author_association: 'CONTRIBUTOR' }) })).writes.length, 2); }); +test('merged contributors may keep ten open PRs but the eleventh closes, including drafts', async () => { + const merged = pr(0, { state: 'closed', pull_request: { merged_at: '2026-09-08T00:00:00Z' } }); + for (let n = 6; n <= 10; n++) { + const history = [merged, ...Array.from({ length: n }, (_, i) => pr(i + 1))]; + assert.deepEqual((await run({ current: pr(n), history })).writes, []); + } + const history = [merged, ...Array.from({ length: 10 }, (_, i) => pr(i + 1))]; + const current = pr(11, { draft: true }); + const { writes, calls } = await run({ current, history }); + assert.deepEqual(writes.map((w) => w.kind), ['comment', 'close']); + assert.match(writes[0].body, /11 open PRs, including this one \(limit: 10\)/); + assert.match(writes[0].body, /Contributors with merged PRs/); + assert.match(writes[0].body, /fewer than 10 of your other PRs/); + assert.ok(calls.every((number) => number === 11)); + assert.deepEqual((await run({ current, history, openHistory: history.slice(1, 10) })).writes, []); + assert.deepEqual((await run({ current, history, env: { DRY_RUN: 'true' } })).writes, []); +}); +test('the established-contributor cap is configurable and invalid values skip safely', async () => { + const history = [pr(0, { state: 'closed', pull_request: { merged_at: '2026-09-08T00:00:00Z' } }), + ...Array.from({ length: 7 }, (_, i) => pr(i + 1))]; + const { writes } = await run({ current: pr(7), history, env: { MAX_OPEN_ESTABLISHED: '6' } }); + assert.match(writes[0].body, /limit: 6/); + for (const value of ['0', '-1', '1.5', 'bad', 'Infinity', '9007199254740992']) { + const result = await run({ env: { MAX_OPEN_ESTABLISHED: value } }); + assert.deepEqual(result.calls, []); + assert.deepEqual(result.writes, []); + assert.equal(result.warnings.length, 1); + } +}); test('maintainers, bots, allowlisted users and labeled PRs are exempt', async () => { for (const permission of ['write', 'admin', 'maintain']) assert.deepEqual((await run({ permission })).writes, []); diff --git a/.github/workflows/pr-rate-limit.yml b/.github/workflows/pr-rate-limit.yml index 9f949c5d3cc0..fb9ed39c4a0f 100644 --- a/.github/workflows/pr-rate-limit.yml +++ b/.github/workflows/pr-rate-limit.yml @@ -13,7 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -name: New Contributor PR Rate Limit +name: Contributor Open PR Limit on: pull_request_target: @@ -53,11 +53,13 @@ jobs: DRY_RUN: ${{ vars.PR_RATE_LIMIT_DRY_RUN }} EXEMPT_USERS: ${{ vars.PR_RATE_LIMIT_EXEMPT_USERS }} MAX_OPEN: ${{ vars.PR_RATE_LIMIT_MAX_OPEN }} + MAX_OPEN_ESTABLISHED: ${{ vars.PR_RATE_LIMIT_MAX_OPEN_ESTABLISHED }} with: script: | - const OPEN_LIMIT = Number(process.env.MAX_OPEN?.trim() || '5'); - if (!Number.isSafeInteger(OPEN_LIMIT) || OPEN_LIMIT < 1) { - core.warning('Invalid PR_RATE_LIMIT_MAX_OPEN; no moderation performed.'); + const NEW_LIMIT = Number(process.env.MAX_OPEN?.trim() || '5'); + const ESTABLISHED_LIMIT = Number(process.env.MAX_OPEN_ESTABLISHED?.trim() || '10'); + if (![NEW_LIMIT, ESTABLISHED_LIMIT].every((limit) => Number.isSafeInteger(limit) && limit >= 1)) { + core.warning('Invalid contributor open-PR cap configuration; no moderation performed.'); return; } const EXEMPT_LABEL = 'pr-rate-limit-exempt'; @@ -114,11 +116,12 @@ jobs: // Include the triggering PR even if its listing has not caught up yet. history.set(pull.number, pull); let openCount = [...history.values()].filter((item) => item.state === 'open').length; - if (openCount <= OPEN_LIMIT) return; // A closed PR is not necessarily a contribution: require an actual merge. // Check history directly rather than trusting author_association or a // search result that may not yet include a recent merge. - if ([...history.values()].some((item) => item.pull_request?.merged_at)) return; + const hasMergedPR = [...history.values()].some((item) => item.pull_request?.merged_at); + const OPEN_LIMIT = hasMergedPR ? ESTABLISHED_LIMIT : NEW_LIMIT; + if (openCount <= OPEN_LIMIT) return; const comments = await github.paginate(github.rest.issues.listComments, { owner, repo, issue_number: pull_number, per_page: 100, }); @@ -134,7 +137,8 @@ jobs: ' open PRs, including this one (limit: ' + OPEN_LIMIT + ')'; const body = [ 'Closing this PR because ' + reason + '.', - 'Contributors with no merged PRs in this repository may keep up to ' + OPEN_LIMIT + ' PRs open, including drafts.', + 'Contributors ' + (hasMergedPR ? 'with merged PRs' : 'with no merged PRs') + + ' in this repository may keep up to ' + OPEN_LIMIT + ' PRs open, including drafts.', 'Please wait until your existing PRs are reviewed or merged before opening more. ' + 'Reviewed PRs still count while open. ' + 'You may ask a maintainer to reopen this PR when fewer than ' + OPEN_LIMIT + ' of your other PRs are open, ' + diff --git a/AGENTS.md b/AGENTS.md index 2861a1dcf093..4f9bc8784c46 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -167,8 +167,8 @@ See [CI overview](docs/source/developer-guide/ci-overview.md) for full details. ### Triggering CI -The new-contributor open-PR cap is implemented in `.github/workflows/pr-rate-limit.yml`; -see `CONTRIBUTING.md` for the default five-open-PR cap, exemptions and manual recovery. Its mocked API tests +The contributor open-PR caps are implemented in `.github/workflows/pr-rate-limit.yml`; +see `CONTRIBUTING.md` for the default five/ten-open-PR caps based on merge history, exemptions and manual recovery. Its mocked API tests run with `node --test .github/scripts/pr_rate_limit.test.js` and are registered in Release Checks. CI is triggered by posting comments on the PR. Basic commands: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c9a0552ca387..15dd344985b7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -118,27 +118,30 @@ For NVIDIA developers, please submit feature or bug fixes to the dedicated bran Meanwhile, please add the "release blocker" label to any PRs that could potentially cause a release delay. -### Open pull requests for new contributors +### Open pull requests for contributors -Contributors with no merged PRs in this repository may keep up to five PRs open at a time, -including drafts. Please wait until your existing PRs are reviewed or merged before opening more. +Contributors with no merged PRs in this repository may keep up to five PRs open at a time. +Contributors with at least one merged PR may keep up to ten open PRs. Both caps include drafts; +a first merge raises the cap instead of removing it. +Please wait until your existing PRs are reviewed or merged before opening more. Reviewed PRs still count while open; closing or merging a PR frees capacity. Excess PRs are automatically closed after creation or reopening. The explanation includes the observed open count and configured cap; retries reuse that comment, so its counts describe the original evaluation. Closed PRs do not consume capacity, and there is no submission cooldown. -Ask a maintainer to reopen a PR when fewer than five other PRs are open. GitHub may prevent authors +Ask a maintainer to reopen a PR when the number of other open PRs is below your applicable cap. GitHub may prevent authors from reopening PRs closed by the bot. Concurrent submissions or reopenings may be closed while the observed open count exceeds the cap, regardless of how old the PRs are. Users with write access and bot accounts are exempt. Maintainers can apply `pr-rate-limit-exempt` to an individual PR before reopening it. Administrators can exempt trusted contributors through the -comma-separated `PR_RATE_LIMIT_EXEMPT_USERS` repository variable, set `PR_RATE_LIMIT_MAX_OPEN` -to a positive integer to override the default cap of five, or set `PR_RATE_LIMIT_DRY_RUN=true` +comma-separated `PR_RATE_LIMIT_EXEMPT_USERS` repository variable. Set `PR_RATE_LIMIT_MAX_OPEN` +to override the newcomer cap of five, or `PR_RATE_LIMIT_MAX_OPEN_ESTABLISHED` to override the +merged-contributor cap of ten; both require positive integers. Set `PR_RATE_LIMIT_DRY_RUN=true` to log decisions without commenting or closing. The workflow does not prevent independently triggered CI jobs from starting. Incomplete history or unavailable permission checks (403/404) produce warnings and skip moderation. Invalid cap configuration also warns and skips. Other API failures fail the job. -Maintainers should monitor warnings and failed **New Contributor PR Rate Limit** runs in the Actions +Maintainers should monitor warnings and failed **Contributor Open PR Limit** runs in the Actions tab. After resolving the reported problem, re-run the job or choose **Run workflow** on the default branch and enter the affected PR number. Manual recovery uses the same exemptions, dry-run setting and cap, and reuses an existing bot comment before retrying closure. Recovery is manual; there is no From 3f60bcc7f57f921b1495b96acb933cdaa41f9bc5 Mon Sep 17 00:00:00 2001 From: Kanghwan Jang <861393+karljang@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:57:05 -0700 Subject: [PATCH 6/6] Serialize PR moderation and simplify contributor guidance Signed-off-by: Kanghwan Jang <861393+karljang@users.noreply.github.com> --- .github/scripts/pr_rate_limit.test.js | 12 +++++++----- .github/workflows/pr-rate-limit.yml | 8 +++++--- AGENTS.md | 17 ++++++++++++++++- CONTRIBUTING.md | 25 ++++++------------------- 4 files changed, 34 insertions(+), 28 deletions(-) diff --git a/.github/scripts/pr_rate_limit.test.js b/.github/scripts/pr_rate_limit.test.js index 7cd03dd382a1..40723db79480 100644 --- a/.github/scripts/pr_rate_limit.test.js +++ b/.github/scripts/pr_rate_limit.test.js @@ -96,9 +96,11 @@ test('first five pass; sixth receives explanation before closure', async () => { assert.equal(writes[1].state, 'closed'); }); test('counts drafts but excludes closed PRs, ordinary issues and another author', async () => { - const history = Array.from({ length: 5 }, (_, i) => pr(i + 1, { draft: true })); - history.push(pr(-2, { state: 'closed' }), pr(0, { pull_request: undefined }), pr(-1, { user: { id: 99 } })); - assert.equal((await run({ history })).writes.length, 2); + const excluded = [pr(-2, { state: 'closed' }), pr(0, { pull_request: undefined }), pr(-1, { user: { id: 99 } })]; + assert.deepEqual((await run({ history: [...Array.from({ length: 4 }, (_, i) => pr(i + 1, { draft: true })), ...excluded] })).writes, []); + const { writes } = await run({ history: [...Array.from({ length: 5 }, (_, i) => pr(i + 1, { draft: true })), ...excluded] }); + assert.deepEqual(writes.map((w) => w.kind), ['comment', 'close']); + assert.match(writes[0].body, /currently have 6 open PRs/); }); @@ -182,12 +184,12 @@ test('successful comment survives a close failure and is not duplicated on retry const comments = [{ user: { login: 'github-actions[bot]' }, body: writes[0].body }]; assert.deepEqual((await run({ comments })).writes.map((w) => w.kind), ['close']); }); -test('workflow uses trusted inline code, minimal permissions and per-PR concurrency', () => { +test('workflow uses trusted inline code, minimal permissions and a shared moderation queue', () => { assert.match(workflow, /pull_request_target:/); assert.match(workflow, /types: \[opened, reopened, ready_for_review\]/); assert.match(workflow, /pull-requests: write/); assert.match(workflow, /issues: read/); - assert.match(workflow, /group: pr-rate-limit-\$\{\{ github.event.pull_request.number \|\| inputs.pr_number \}\}/); + assert.match(workflow, /concurrency:\n group: pr-rate-limit\n cancel-in-progress: false\n queue: max/); assert.doesNotMatch(workflow, /actions\/checkout|head.sha|secrets\./); }); diff --git a/.github/workflows/pr-rate-limit.yml b/.github/workflows/pr-rate-limit.yml index fb9ed39c4a0f..4e81a74c9d16 100644 --- a/.github/workflows/pr-rate-limit.yml +++ b/.github/workflows/pr-rate-limit.yml @@ -30,11 +30,13 @@ permissions: pull-requests: write issues: read -# Serialize retries for one PR, not all submissions by an author: a shared -# author group can discard pending runs during the very burst we must handle. +# Serialize all moderation, including manual recovery, so separate PRs cannot +# close against the same capacity snapshot. queue: max retains up to 100 pending +# runs; monitor canceled runs and manually reevaluate affected PRs on overflow. concurrency: - group: pr-rate-limit-${{ github.event.pull_request.number || inputs.pr_number }} + group: pr-rate-limit cancel-in-progress: false + queue: max jobs: rate-limit: diff --git a/AGENTS.md b/AGENTS.md index 4f9bc8784c46..449a5ed2270f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -168,9 +168,24 @@ See [CI overview](docs/source/developer-guide/ci-overview.md) for full details. ### Triggering CI The contributor open-PR caps are implemented in `.github/workflows/pr-rate-limit.yml`; -see `CONTRIBUTING.md` for the default five/ten-open-PR caps based on merge history, exemptions and manual recovery. Its mocked API tests +see `CONTRIBUTING.md` for the default five/ten-open-PR caps based on merge history and exemptions. Its mocked API tests run with `node --test .github/scripts/pr_rate_limit.test.js` and are registered in Release Checks. +Administrators can set `PR_RATE_LIMIT_MAX_OPEN` (default five) and +`PR_RATE_LIMIT_MAX_OPEN_ESTABLISHED` (default ten) to positive integers, and exempt trusted +logins through the comma-separated `PR_RATE_LIMIT_EXEMPT_USERS` repository variable. +Set `PR_RATE_LIMIT_DRY_RUN=true` to log decisions without commenting or closing. +Incomplete history, invalid caps and permission-check 403/404 responses warn and skip moderation; +other API failures fail the job. Independent CI jobs are not gated by this workflow. + +All moderation runs share a serial queue, including default-branch manual dispatch. +The queue holds up to 100 pending runs; overflow cancels additional runs. Monitor warnings, +failures and canceled **Contributor Open PR Limit** runs in Actions. After resolving a problem, +rerun the job or choose **Run workflow** on the default branch and enter the affected PR number. +Recovery uses the same exemptions, dry-run setting and cap. It reuses the original bot comment, +whose count reflects the first evaluation. Recovery is manual; there is no scheduled sweep or +instantaneous-cap guarantee while runs are queued or GitHub is unavailable. + CI is triggered by posting comments on the PR. Basic commands: - `/bot run` — trigger the standard CI pipeline - `/bot run --disable-fail-fast` — run all stages even if earlier ones fail (only add when explicitly needed) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 15dd344985b7..120d43727cf9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -118,34 +118,21 @@ For NVIDIA developers, please submit feature or bug fixes to the dedicated bran Meanwhile, please add the "release blocker" label to any PRs that could potentially cause a release delay. -### Open pull requests for contributors +### Open pull request limits Contributors with no merged PRs in this repository may keep up to five PRs open at a time. Contributors with at least one merged PR may keep up to ten open PRs. Both caps include drafts; a first merge raises the cap instead of removing it. Please wait until your existing PRs are reviewed or merged before opening more. Reviewed PRs still count while open; closing or merging a PR frees capacity. -Excess PRs are automatically closed after creation or reopening. The explanation includes the -observed open count and configured cap; retries reuse that comment, so its counts describe the -original evaluation. Closed PRs do not consume capacity, and there is no submission cooldown. +When enforcement is enabled, excess PRs are automatically closed after creation or reopening. +While dry-run mode is enabled, the workflow only logs decisions and does not close PRs. +Closed PRs do not consume capacity, and there is no submission cooldown. Ask a maintainer to reopen a PR when the number of other open PRs is below your applicable cap. GitHub may prevent authors -from reopening PRs closed by the bot. Concurrent submissions or reopenings may be closed while -the observed open count exceeds the cap, regardless of how old the PRs are. +from reopening PRs closed by the bot. Users with write access and bot accounts are exempt. Maintainers can apply `pr-rate-limit-exempt` -to an individual PR before reopening it. Administrators can exempt trusted contributors through the -comma-separated `PR_RATE_LIMIT_EXEMPT_USERS` repository variable. Set `PR_RATE_LIMIT_MAX_OPEN` -to override the newcomer cap of five, or `PR_RATE_LIMIT_MAX_OPEN_ESTABLISHED` to override the -merged-contributor cap of ten; both require positive integers. Set `PR_RATE_LIMIT_DRY_RUN=true` -to log decisions without commenting or closing. The workflow does not prevent independently triggered -CI jobs from starting. Incomplete history or unavailable permission checks (403/404) produce warnings -and skip moderation. Invalid cap configuration also warns and skips. Other API failures fail the job. - -Maintainers should monitor warnings and failed **Contributor Open PR Limit** runs in the Actions -tab. After resolving the reported problem, re-run the job or choose **Run workflow** on the default -branch and enter the affected PR number. Manual recovery uses the same exemptions, dry-run setting -and cap, and reuses an existing bot comment before retrying closure. Recovery is manual; there is no -scheduled sweep or guarantee of an instantaneous cap while jobs are pending or GitHub is unavailable. +to an individual PR before reopening it, or arrange a trusted-contributor exception. ### Inactive pull requests