Browse Source

fix: cap review owners per module

Turtle 1 month ago
parent
commit
0b43d89d5f

+ 2 - 2
.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: dde97002f8a138687ecfc1c8d0bfc9a373ef29a5
-2026-09-08-trusted-changed-file-review-routing.zh.md: 2d618d5ebbcaf46f3a6ac4bbfbb80f51e516323b
+2026-09-08-trusted-changed-file-review-routing.md: 35374b883449a40a94431c0deb7efa0906bc9b72
+2026-09-08-trusted-changed-file-review-routing.zh.md: 117cbbce5490e54aba6c0aa6514e176d043a36bf

+ 2 - 2
.agents/notes/implemented/process/2026-09-08-trusted-changed-file-review-routing.md

@@ -12,7 +12,7 @@ Review routing needs an observable changed-file input, explicit owner rules, com
 
 ## Decision
 
-The repository keeps a CODEOWNERS-compatible map at [`.github/review-ownership/CODEOWNERS`](../../../../.github/review-ownership/CODEOWNERS), outside GitHub's native CODEOWNERS locations. The map accepts only explicit absolute directory patterns and individual GitHub users. It rejects wildcards, hidden-directory patterns, teams, duplicate patterns, and duplicate owners. Later matching patterns replace earlier matches.
+The repository keeps a CODEOWNERS-compatible map at [`.github/review-ownership/CODEOWNERS`](../../../../.github/review-ownership/CODEOWNERS), outside GitHub's native CODEOWNERS locations. The map accepts only explicit absolute directory patterns with one or two individual GitHub users. It rejects wildcards, hidden-directory patterns, teams, more than two owners, duplicate patterns, and duplicate owners. Later matching patterns replace earlier matches.
 
 The policy test counts non-test tracked lines in directories that match an ownership rule. It rejects a map in which `@turtle1999` owns more than one third of that eligible owned codebase.
 
@@ -42,6 +42,6 @@ The workflow prints the changed code paths, each exclusion class, per-file owner
 
 Reviewer mutations are reproducible from a trusted policy, the file classifications printed in the workflow log, and review-request provenance in the pull-request timeline. Excluded changes do not request owners, and draft pull requests do not retain workflow-authored requests. Ownership changes become effective only after merge, so the pull request that changes policy cannot apply its untrusted policy to itself.
 
-The workflow requests every matched owner rather than choosing one owner nondeterministically. Shared ownership on large directories therefore produces multiple requests. GitHub-generated review-request events may not start other workflows that depend on recursively triggered events from `GITHUB_TOKEN`; those workflows must not rely on this request as their only trigger.
+The workflow requests every matched owner rather than choosing one owner nondeterministically. Shared ownership therefore produces at most two requests for each changed module. GitHub-generated review-request events may not start other workflows that depend on recursively triggered events from `GITHUB_TOKEN`; those workflows must not rely on this request as their only trigger.
 
 Any change that does not match an explicit exclusion remains eligible under an owned directory. Unmatched paths are logged and request nobody. Pull requests above the file or timeline API limit fail without applying a partial reviewer mutation.

+ 2 - 2
.agents/notes/implemented/process/2026-09-08-trusted-changed-file-review-routing.zh.md

@@ -12,7 +12,7 @@ Status: implemented
 
 ## 决策
 
-仓库在 GitHub 原生 CODEOWNERS 路径之外的 [`.github/review-ownership/CODEOWNERS`](../../../../.github/review-ownership/CODEOWNERS) 中保存兼容 CODEOWNERS 格式的映射。该映射只接受显式绝对目录模式和 GitHub 个人用户。通配符、隐藏目录模式、团队、重复模式和重复 owner 都会被拒绝。靠后的匹配模式会替换靠前的匹配结果。
+仓库在 GitHub 原生 CODEOWNERS 路径之外的 [`.github/review-ownership/CODEOWNERS`](../../../../.github/review-ownership/CODEOWNERS) 中保存兼容 CODEOWNERS 格式的映射。该映射只接受显式绝对目录模式,每条模式配置一至两名 GitHub 个人用户。通配符、隐藏目录模式、团队、超过两名 owner、重复模式和重复 owner 都会被拒绝。靠后的匹配模式会替换靠前的匹配结果。
 
 策略测试会统计匹配所有权规则的目录中的非测试跟踪文件行数。如果 `@turtle1999` 拥有的有效代码库超过三分之一,测试就会拒绝该映射。
 
@@ -42,6 +42,6 @@ Workflow 会在发出任何评审请求变更之前,依次打印变更代码
 
 评审请求修改可以根据受信任的策略、workflow 日志中打印的文件分类,以及 PR 时间线中的请求来源复现。被排除的变更不会请求 owner,草稿 PR 不会保留 workflow 发出的请求。所有权变更只有合并后才会生效,因此修改策略的 PR 无法对自身应用其中不受信任的策略。
 
-Workflow 会请求所有匹配的 owner,不会随机选择一人。因此,大目录上的共享所有权会产生多个请求。GitHub 使用 `GITHUB_TOKEN` 生成的评审请求事件可能不会启动依赖递归触发事件的其他 workflow;这些 workflow 不得把此请求作为唯一触发条件。
+Workflow 会请求所有匹配的 owner,不会随机选择一人。因此,共享所有权对每个变更模块最多产生两个请求。GitHub 使用 `GITHUB_TOKEN` 生成的评审请求事件可能不会启动依赖递归触发事件的其他 workflow;这些 workflow 不得把此请求作为唯一触发条件。
 
 已分配目录下不符合任何显式排除规则的变更仍符合请求条件。未匹配的路径会被记录,但不会请求任何人。超过文件或时间线 API 上限的 PR 会失败,并且不会执行不完整的评审者修改。

+ 2 - 2
.github/review-ownership/CODEOWNERS

@@ -18,7 +18,7 @@
 /packages/code-runtime/ @Chinesezjc
 /packages/compaction/ @imccyu
 /packages/context/ @turtle1999
-/packages/core/ @tianyicui @turtle1999 @mektpoy
+/packages/core/ @turtle1999 @mektpoy
 /packages/credentials/ @mektpoy
 /packages/e2b/ @mektpoy
 /packages/experimental/ @mektpoy
@@ -41,7 +41,7 @@
 /packages/sandbox/ @mektpoy
 /packages/schedule/ @imccyu
 /packages/sdk/ @mektpoy
-/packages/session/ @tianyicui @turtle1999 @mektpoy
+/packages/session/ @turtle1999 @mektpoy
 /packages/session-query/ @mektpoy
 /packages/settings/ @mektpoy
 /packages/shell/ @mektpoy

+ 2 - 2
.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: fff5516fb6e67b639ffaa4799a040040184dd28a
-README.zh.md: 5b6cb2b60a9b08dca263c9254722b89cd0b225a0
+README.md: 4b22b8d1b0d3a3fb4858b885e7d2e7a3d9a962e8
+README.zh.md: 13eb1abd40e9cdf3f74a9c5b164a2828481c8a49

+ 1 - 1
.github/review-ownership/README.md

@@ -22,7 +22,7 @@ Pull requests run the workflow when opened, synchronized, reopened, marked ready
 
 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.
 
-The ownership map accepts explicit absolute directory patterns and individual GitHub users. It rejects wildcards, hidden-directory patterns, teams, 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.
+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.
 
 The policy test measures non-test tracked lines under matched directories and requires `@turtle1999` to own no more than one third of that eligible owned codebase.
 

+ 1 - 1
.github/review-ownership/README.zh.md

@@ -22,7 +22,7 @@ PR 在创建、同步、重新打开、标记为可评审或转为草稿时运
 
 对于非草稿 PR,workflow 会请求尚未加入的匹配 owner。对于草稿,workflow 会读取当前评审请求和评审请求时间线,然后取消最近一次请求者为 `github-actions[bot]` 的当前请求。由人员发出的当前请求保持不变。如果时间线超过 3,000 个事件或包含无效的请求来源,workflow 会失败且不执行取消操作。
 
-所有权映射只接受显式绝对目录模式和 GitHub 个人用户。通配符、隐藏目录模式、团队,以及重复的模式或 owner 都会被拒绝。匹配遵循 CODEOWNERS 的最后一条匹配规则。扫描器会在修改评审请求前,打印变更代码文件、排除的测试文件、文档文件和纯注释文件,逐文件 owner 匹配结果,以及将要请求或取消的评审者。未匹配的文件仍显示在日志中。PR 作者和已经收到评审请求的用户不会收到新请求。
+所有权映射只接受显式绝对目录模式,并允许每条模式配置一至两名 GitHub 个人用户。通配符、隐藏目录模式、团队、超过两名 owner,以及重复的模式或 owner 都会被拒绝。匹配遵循 CODEOWNERS 的最后一条匹配规则。扫描器会在修改评审请求前,打印变更代码文件、排除的测试文件、文档文件和纯注释文件,逐文件 owner 匹配结果,以及将要请求或取消的评审者。未匹配的文件仍显示在日志中。PR 作者和已经收到评审请求的用户不会收到新请求。
 
 策略测试会统计已匹配目录下的非测试跟踪文件行数,并要求 `@turtle1999` 拥有的有效代码库不超过三分之一。
 

+ 4 - 0
.github/review-ownership/request-review.mjs

@@ -5,6 +5,7 @@ import process from 'node:process'
 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_TIMELINE_EVENTS = 3_000
 const PAGE_SIZE = 100
@@ -40,6 +41,9 @@ export function parseOwnership(source) {
     if (pattern.startsWith('/.')) throw new Error(`${location}: hidden-directory patterns are not allowed`)
     if (patterns.has(pattern)) throw new Error(`${location}: duplicate pattern ${JSON.stringify(pattern)}`)
     if (owners.length === 0) throw new Error(`${location}: expected at least one owner`)
+    if (owners.length > MAX_OWNERS_PER_RULE) {
+      throw new Error(`${location}: expected at most ${MAX_OWNERS_PER_RULE} owners`)
+    }
     const normalizedOwners = []
     const seenOwners = new Set()
     for (const owner of owners) {

+ 7 - 9
.github/review-ownership/request-review.test.mjs

@@ -38,19 +38,16 @@ test('loads the repository ownership policy without test-only directory rules',
   assert.equal(rules.some(rule => rule.pattern === '/packages/test-support/'), false)
   assert.deepEqual(ownersByPattern.get('/apps/cli/'), ['@turtle1999'])
   assert.deepEqual(ownersByPattern.get('/docs/'), ['@turtle1999'])
-  assert.deepEqual(ownersByPattern.get('/packages/core/'), ['@tianyicui', '@turtle1999', '@mektpoy'])
+  assert.deepEqual(ownersByPattern.get('/packages/core/'), ['@turtle1999', '@mektpoy'])
   assert.deepEqual(ownersByPattern.get('/packages/llm/'), ['@LegGasai'])
   assert.deepEqual(ownersByPattern.get('/packages/preset/'), ['@LegGasai', '@turtle1999'])
-  assert.deepEqual(ownersByPattern.get('/packages/session/'), ['@tianyicui', '@turtle1999', '@mektpoy'])
+  assert.deepEqual(ownersByPattern.get('/packages/session/'), ['@turtle1999', '@mektpoy'])
   assert.deepEqual(ownersByPattern.get('/packages/subagent/'), ['@Dudu-0223'])
   assert.deepEqual(ownersByPattern.get('/packages/web/'), ['@imccyu'])
   assert.deepEqual(ownersByPattern.get('/python/'), ['@LegGasai'])
   assert.deepEqual(ownersByPattern.get('/website/'), ['@LegGasai'])
-  assert.deepEqual(
-    rules.filter(rule => rule.owners.includes('@tianyicui')).map(rule => rule.pattern),
-    ['/packages/core/', '/packages/session/'],
-  )
-  for (const excludedOwner of ['@kermeanx', '@pkh-xht']) {
+  assert.equal(rules.every(rule => rule.owners.length <= 2), true)
+  for (const excludedOwner of ['@tianyicui', '@kermeanx', '@pkh-xht']) {
     assert.equal(rules.some(rule => rule.owners.some(owner => owner.toLowerCase() === excludedOwner)), false)
   }
 })
@@ -87,6 +84,7 @@ test('rejects ownership forms the requester cannot apply safely', () => {
     ['/packages/*/ @owner\n', /explicit absolute directory/u],
     ['/packages/core/\n', /at least one owner/u],
     ['/packages/core/ @org/team\n', /individual GitHub users/u],
+    ['/packages/core/ @one @two @three\n', /at most 2 owners/u],
     ['/packages/core/ @owner @OWNER\n', /duplicate owner/u],
     ['/packages/core/ @owner\n/packages/core/ @other\n', /duplicate pattern/u],
   ]) {
@@ -342,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', 'tianyicui'],
+    requestedReviewers: ['Dudu-0223', 'LegGasai', 'mektpoy'],
     cancelledReviewers: [],
   })
   assert.equal(trace[0].type, 'log')
@@ -355,7 +353,7 @@ 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', 'tianyicui'] },
+      body: { reviewers: ['Dudu-0223', 'LegGasai', 'mektpoy'] },
     },
   })
 })