From 7d8546b4db3e4a10d48c48b8bf0a8fa3a265cd95 Mon Sep 17 00:00:00 2001 From: Turtle Date: Tue, 8 Sep 2026 14:37:54 +0800 Subject: [PATCH] fix: cap reviewers per pull request --- ...sted-changed-file-review-routing.i18n.yaml | 4 +-- ...-08-trusted-changed-file-review-routing.md | 2 +- ...-trusted-changed-file-review-routing.zh.md | 2 +- .github/review-ownership/README.i18n.yaml | 4 +-- .github/review-ownership/README.md | 2 +- .github/review-ownership/README.zh.md | 2 +- .github/review-ownership/request-review.mjs | 6 +++- .../review-ownership/request-review.test.mjs | 28 +++++++++++++++++-- 8 files changed, 38 insertions(+), 12 deletions(-) diff --git a/.agents/notes/implemented/process/2026-09-08-trusted-changed-file-review-routing.i18n.yaml b/.agents/notes/implemented/process/2026-09-08-trusted-changed-file-review-routing.i18n.yaml index 28ef25b93b..5d2b04943a 100644 --- a/.agents/notes/implemented/process/2026-09-08-trusted-changed-file-review-routing.i18n.yaml +++ b/.agents/notes/implemented/process/2026-09-08-trusted-changed-file-review-routing.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/process/2026-09-08-trusted-changed-file-review-routing.md -2026-09-08-trusted-changed-file-review-routing.md: 35374b883449a40a94431c0deb7efa0906bc9b72 -2026-09-08-trusted-changed-file-review-routing.zh.md: 117cbbce5490e54aba6c0aa6514e176d043a36bf +2026-09-08-trusted-changed-file-review-routing.md: 9f59f336fb1f8d2fdd1e13f6751258a47518b4dc +2026-09-08-trusted-changed-file-review-routing.zh.md: e845c79051858c11053597516f486cf29fa2805a diff --git a/.agents/notes/implemented/process/2026-09-08-trusted-changed-file-review-routing.md b/.agents/notes/implemented/process/2026-09-08-trusted-changed-file-review-routing.md index 35374b8834..9f59f336fb 100644 --- a/.agents/notes/implemented/process/2026-09-08-trusted-changed-file-review-routing.md +++ b/.agents/notes/implemented/process/2026-09-08-trusted-changed-file-review-routing.md @@ -22,7 +22,7 @@ The scanner fetches every changed-file record before deciding. It fails if the p The scanner excludes test-only paths before owner matching. Excluded paths comprise directories named `test`, `tests`, `__tests__`, `__snapshots__`, `benches`, or `stress-tests`; the top-level `benchmarks` and `snapshots` trees; `packages/test-support`; `scripts/fixtures` and `scripts/snapshots`; filenames ending in `.bench.`, `.corpus.`, `.e2e.`, `.perf.`, `.snapshot.`, `.spec.`, `.stress.`, or `.test.`; and Python `test_*.py`, `*_test.py`, or `*_tests.py` files. Test infrastructure such as `vitest*.config.ts` and gate implementations remains reviewable because it changes how repository evidence is produced. The [comment-only routing decision](2026-09-08-comment-only-review-routing.md) owns the additional documentation and comment exclusions. -The workflow prints the changed code paths, each exclusion class, per-file owner matches, and final reviewer action before any review-request mutation. For a non-draft pull request, it requests the union of matched individual owners after removing the pull-request author and users who are already requested. For a draft, it reads the complete review-request timeline and cancels current requests whose latest requester is `github-actions[bot]`; requests made by people remain unchanged. +The workflow prints the changed code paths, each exclusion class, per-file owner matches, and final reviewer action before any review-request mutation. For a non-draft pull request, it removes the pull-request author and users who are already requested from the login-sorted union of matched individual owners, then fills at most two current individual review-request slots. Existing individual requests consume slots even when they do not match the ownership map. Login order deterministically selects candidates when the available slots cannot cover the remaining union. The workflow does not remove requests from a non-draft pull request. For a draft, it reads the complete review-request timeline and cancels current requests whose latest requester is `github-actions[bot]`; requests made by people remain unchanged. ## Verification diff --git a/.agents/notes/implemented/process/2026-09-08-trusted-changed-file-review-routing.zh.md b/.agents/notes/implemented/process/2026-09-08-trusted-changed-file-review-routing.zh.md index 117cbbce54..e845c79051 100644 --- a/.agents/notes/implemented/process/2026-09-08-trusted-changed-file-review-routing.zh.md +++ b/.agents/notes/implemented/process/2026-09-08-trusted-changed-file-review-routing.zh.md @@ -22,7 +22,7 @@ Status: implemented 扫描器会在匹配 owner 之前排除纯测试路径。排除范围包括名为 `test`、`tests`、`__tests__`、`__snapshots__`、`benches` 或 `stress-tests` 的目录，顶层 `benchmarks` 和 `snapshots` 目录树，`packages/test-support`、`scripts/fixtures` 和 `scripts/snapshots`，以 `.bench.`、`.corpus.`、`.e2e.`、`.perf.`、`.snapshot.`、`.spec.`、`.stress.` 或 `.test.` 结尾的文件名，以及 Python 的 `test_*.py`、`*_test.py` 或 `*_tests.py` 文件。`vitest*.config.ts` 和门禁实现等测试基础设施仍需评审，因为它们会改变仓库证据的生成方式。[纯注释路由决策](2026-09-08-comment-only-review-routing.zh.md)记录额外的文档和注释排除规则。 -Workflow 会在发出任何评审请求变更之前，依次打印变更代码路径、每类排除项、逐文件 owner 匹配结果和最终评审操作。对于非草稿 PR，它会合并匹配到的个人 owner，并排除 PR 作者和已经收到评审请求的用户。对于草稿，它会读取完整的评审请求时间线，并取消最近一次请求者为 `github-actions[bot]` 的当前请求；由人员发出的请求保持不变。 +Workflow 会在发出任何评审请求变更之前，依次打印变更代码路径、每类排除项、逐文件 owner 匹配结果和最终评审操作。对于非草稿 PR，它会从按登录名排序并合并的个人 owner 中排除 PR 作者和已经收到评审请求的用户，然后填充最多两个当前个人评审请求名额。现有个人请求即使不匹配所有权映射，也会占用名额。当可用名额无法覆盖剩余候选集合时，登录名顺序会确定性地选择候选人。Workflow 不会从非草稿 PR 移除请求。对于草稿，它会读取完整的评审请求时间线，并取消最近一次请求者为 `github-actions[bot]` 的当前请求；由人员发出的请求保持不变。 ## 验证 diff --git a/.github/review-ownership/README.i18n.yaml b/.github/review-ownership/README.i18n.yaml index 99b0847a23..553d8a0d39 100644 --- a/.github/review-ownership/README.i18n.yaml +++ b/.github/review-ownership/README.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .github/review-ownership/README.md -README.md: 4b22b8d1b0d3a3fb4858b885e7d2e7a3d9a962e8 -README.zh.md: 13eb1abd40e9cdf3f74a9c5b164a2828481c8a49 +README.md: 4c412a7c47f114e0aa6aa9a3048fd7f833f0b0e3 +README.zh.md: 1db7d1b56d4927a83c502557f393cb6518dc09f1 diff --git a/.github/review-ownership/README.md b/.github/review-ownership/README.md index 4b22b8d1b0..4c412a7c47 100644 --- a/.github/review-ownership/README.md +++ b/.github/review-ownership/README.md @@ -20,7 +20,7 @@ The [`request-review` workflow](../workflows/request-review.yml) reads the CODEO Pull requests run the workflow when opened, synchronized, reopened, marked ready for review, or converted to a draft. The scanner fetches the complete pull-request file list, evaluates both paths of a rename, and fails instead of routing from a partial list. GitHub exposes at most 3,000 files for this API. -For a non-draft pull request, the workflow requests missing matched owners. For a draft, it reads the current requested reviewers and review-request timeline, then cancels each current request whose latest requester is `github-actions[bot]`. Current requests made by people remain unchanged. The workflow fails without cancellation when the timeline exceeds 3,000 events or contains invalid request provenance. +For a non-draft pull request, the workflow requests missing matched owners while keeping the total number of current individual review requests at two or fewer. Existing individual requests consume those slots, including requests made by people outside the ownership map. When more candidates remain than available slots, login order selects the reviewers deterministically. The workflow does not remove requests from a non-draft pull request. For a draft, it reads the current requested reviewers and review-request timeline, then cancels each current request whose latest requester is `github-actions[bot]`. Current requests made by people remain unchanged. The workflow fails without cancellation when the timeline exceeds 3,000 events or contains invalid request provenance. The ownership map accepts explicit absolute directory patterns and one or two individual GitHub users per pattern. It rejects wildcards, hidden-directory patterns, teams, more than two owners, and duplicate patterns or owners. Matching follows CODEOWNERS last-match semantics. The scanner prints the changed code, excluded test, documentation, and comment-only files; per-file owner matches; and the reviewers it will request or cancel before it mutates review requests. Unmatched files remain visible in the log. The pull-request author and users who are already requested are omitted from new requests. diff --git a/.github/review-ownership/README.zh.md b/.github/review-ownership/README.zh.md index 13eb1abd40..1db7d1b56d 100644 --- a/.github/review-ownership/README.zh.md +++ b/.github/review-ownership/README.zh.md @@ -20,7 +20,7 @@ PR 在创建、同步、重新打开、标记为可评审或转为草稿时运行该 workflow。扫描器获取完整的 PR 文件列表，分别检查重命名前后的路径；如果只能取得部分列表，则停止执行，不发出评审请求。GitHub 对此 API 最多公开 3,000 个文件。 -对于非草稿 PR，workflow 会请求尚未加入的匹配 owner。对于草稿，workflow 会读取当前评审请求和评审请求时间线，然后取消最近一次请求者为 `github-actions[bot]` 的当前请求。由人员发出的当前请求保持不变。如果时间线超过 3,000 个事件或包含无效的请求来源，workflow 会失败且不执行取消操作。 +对于非草稿 PR，workflow 会请求尚未加入的匹配 owner，同时确保当前个人评审请求总数不超过两个。现有个人请求会占用名额，包括由人员向所有权映射之外用户发出的请求。当剩余候选人数超过可用名额时，workflow 会按登录名顺序确定评审者。Workflow 不会从非草稿 PR 移除请求。对于草稿，workflow 会读取当前评审请求和评审请求时间线，然后取消最近一次请求者为 `github-actions[bot]` 的当前请求。由人员发出的当前请求保持不变。如果时间线超过 3,000 个事件或包含无效的请求来源，workflow 会失败且不执行取消操作。 所有权映射只接受显式绝对目录模式，并允许每条模式配置一至两名 GitHub 个人用户。通配符、隐藏目录模式、团队、超过两名 owner，以及重复的模式或 owner 都会被拒绝。匹配遵循 CODEOWNERS 的最后一条匹配规则。扫描器会在修改评审请求前，打印变更代码文件、排除的测试文件、文档文件和纯注释文件，逐文件 owner 匹配结果，以及将要请求或取消的评审者。未匹配的文件仍显示在日志中。PR 作者和已经收到评审请求的用户不会收到新请求。 diff --git a/.github/review-ownership/request-review.mjs b/.github/review-ownership/request-review.mjs index 39a38d00bc..0f17e783f0 100644 --- a/.github/review-ownership/request-review.mjs +++ b/.github/review-ownership/request-review.mjs @@ -7,6 +7,7 @@ import { pathToFileURL } from 'node:url' const API_VERSION = '2026-03-10' const MAX_OWNERS_PER_RULE = 2 const MAX_PULL_REQUEST_FILES = 3_000 +const MAX_REQUESTED_REVIEWERS = 2 const MAX_TIMELINE_EVENTS = 3_000 const PAGE_SIZE = 100 const WORKFLOW_REVIEW_REQUESTER = 'github-actions[bot]' @@ -460,7 +461,10 @@ export async function requestReviews({ event, ownershipSource, api, write = line } const existing = await api(`/repos/${pull.repository}/pulls/${pull.number}/requested_reviewers`) const alreadyRequested = new Set(requestedReviewerLogins(existing).map(login => login.toLowerCase())) - const reviewers = candidates.filter(login => !alreadyRequested.has(login.toLowerCase())) + const availableSlots = Math.max(0, MAX_REQUESTED_REVIEWERS - alreadyRequested.size) + const reviewers = candidates + .filter(login => !alreadyRequested.has(login.toLowerCase())) + .slice(0, availableSlots) writeList(write, 'Reviewers to request', reviewers.map(login => `@${login}`)) if (reviewers.length === 0) return { ...classified, requestedReviewers: [], cancelledReviewers: [] } diff --git a/.github/review-ownership/request-review.test.mjs b/.github/review-ownership/request-review.test.mjs index e14e61e48f..730e7a81f7 100644 --- a/.github/review-ownership/request-review.test.mjs +++ b/.github/review-ownership/request-review.test.mjs @@ -303,7 +303,7 @@ test('fails closed when the review-request timeline exceeds its limit', async () assert.equal(calls, 30) }) -test('prints changed code files before requesting missing owners', async () => { +test('prints changed code files and limits current review requests to two people', async () => { const trace = [] const files = [ { filename: 'packages/core/agent/src/index.ts' }, @@ -340,7 +340,7 @@ test('prints changed code files before requesting missing owners', async () => { excludedTestFiles: ['packages/core/agent/tests/index.spec.ts'], excludedDocumentationFiles: ['AGENTS.md'], excludedCommentOnlyFiles: [], - requestedReviewers: ['Dudu-0223', 'LegGasai', 'mektpoy'], + requestedReviewers: ['Dudu-0223'], cancelledReviewers: [], }) assert.equal(trace[0].type, 'log') @@ -353,11 +353,33 @@ test('prints changed code files before requesting missing owners', async () => { path: '/repos/deepseek-harness/deepseek-harness/pulls/42/requested_reviewers', options: { method: 'POST', - body: { reviewers: ['Dudu-0223', 'LegGasai', 'mektpoy'] }, + body: { reviewers: ['Dudu-0223'] }, }, }) }) +test('does not add an owner when two people are already requested', async () => { + const calls = [] + const result = await requestReviews({ + event: pullRequestEvent(), + ownershipSource: '/packages/core/ @turtle1999 @mektpoy\n', + api: async (path, options = {}) => { + calls.push({ path, options }) + if (path.endsWith('/files?per_page=100&page=1')) { + return [{ filename: 'packages/core/agent/src/index.ts' }] + } + if (path.endsWith('/requested_reviewers') && options.method === undefined) { + return { users: [{ login: 'first' }, { login: 'second' }], teams: [] } + } + throw new Error(`unexpected API path ${path}`) + }, + write: () => {}, + }) + + assert.deepEqual(result.requestedReviewers, []) + assert.equal(calls.some(call => call.options.method === 'POST'), false) +}) + test('does not request reviewers for test, documentation, or comment-only changes', async () => { const calls = [] const output = []