mirror of
https://github.com/deepseek-ai/deepseek-harness.git
synced 2026-09-13 04:03:30 +00:00
Merge pull request #3684 from deepseek-harness/turtle/custom-review-ownership
chore: automate changed-file review ownership
This commit is contained in:
@@ -0,0 +1,60 @@
|
||||
# Custom static-scanner input. Its nested path keeps GitHub from loading it as
|
||||
# the repository's native CODEOWNERS file.
|
||||
/apps/cli/ @turtle1999
|
||||
/apps/web/ @imccyu
|
||||
/docs/ @turtle1999
|
||||
/native/ @mektpoy
|
||||
/patches/ @mektpoy
|
||||
/python/ @LegGasai
|
||||
/scripts/ @turtle1999
|
||||
/vendor/ @turtle1999
|
||||
/website/ @LegGasai
|
||||
/packages/acp/ @mektpoy
|
||||
/packages/api/ @imccyu
|
||||
/packages/attachment/ @CreatixChu
|
||||
/packages/boot/ @turtle1999
|
||||
/packages/bundle/ @turtle1999
|
||||
/packages/client/ @imccyu
|
||||
/packages/code-runtime/ @Chinesezjc
|
||||
/packages/compaction/ @imccyu
|
||||
/packages/context/ @turtle1999
|
||||
/packages/core/ @tianyicui @turtle1999 @mektpoy
|
||||
/packages/credentials/ @mektpoy
|
||||
/packages/e2b/ @mektpoy
|
||||
/packages/experimental/ @mektpoy
|
||||
/packages/extensions/ @mektpoy
|
||||
/packages/feedback/ @mektpoy
|
||||
/packages/fs/ @mektpoy
|
||||
/packages/goal/ @mektpoy
|
||||
/packages/guard/ @turtle1999
|
||||
/packages/hooks/ @mektpoy
|
||||
/packages/host/ @turtle1999
|
||||
/packages/identity/ @imccyu
|
||||
/packages/interaction/ @imccyu
|
||||
/packages/jobs/ @imccyu
|
||||
/packages/llm/ @LegGasai
|
||||
/packages/lsp/ @mektpoy
|
||||
/packages/mcp/ @mektpoy
|
||||
/packages/plan/ @mektpoy
|
||||
/packages/preset/ @LegGasai @turtle1999
|
||||
/packages/runtime-diagnostics/ @mektpoy
|
||||
/packages/sandbox/ @mektpoy
|
||||
/packages/schedule/ @imccyu
|
||||
/packages/sdk/ @mektpoy
|
||||
/packages/session/ @tianyicui @turtle1999 @mektpoy
|
||||
/packages/session-query/ @mektpoy
|
||||
/packages/settings/ @mektpoy
|
||||
/packages/shell/ @mektpoy
|
||||
/packages/skill/ @mektpoy
|
||||
/packages/spill/ @mektpoy
|
||||
/packages/storage/ @imccyu
|
||||
/packages/subagent/ @Dudu-0223
|
||||
/packages/subprocess/ @mektpoy
|
||||
/packages/terminal/ @imccyu
|
||||
/packages/todo/ @mektpoy
|
||||
/packages/typert/ @imccyu
|
||||
/packages/util/ @mektpoy
|
||||
/packages/web/ @imccyu
|
||||
/packages/webhook/ @mektpoy
|
||||
/packages/workflow/ @mektpoy
|
||||
/packages/workspace/ @imccyu
|
||||
@@ -0,0 +1,6 @@
|
||||
# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each
|
||||
# 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: 95dd52595bacb9b2b00bc475966f328b06098a14
|
||||
README.zh.md: a20a5f4ae41b5d0418601afa853ac2fbc141cc96
|
||||
@@ -0,0 +1,53 @@
|
||||
# Automated review requests
|
||||
|
||||
English | [中文](README.zh.md)
|
||||
|
||||
## Summary
|
||||
|
||||
The [`request-review` workflow](../workflows/request-review.yml) reads the CODEOWNERS-compatible [ownership map](CODEOWNERS) from the trusted default branch. It prints the complete changed non-test file list, matches those files to owners, and then requests the missing reviewers. The ownership map is outside GitHub's native CODEOWNERS locations, so GitHub does not apply it directly.
|
||||
|
||||
## Table of Contents
|
||||
|
||||
- [Routing](#routing)
|
||||
- [Test exclusion](#test-exclusion)
|
||||
- [Security](#security)
|
||||
- [Verification](#verification)
|
||||
- [Dev Note](#dev-note)
|
||||
|
||||
<a id="routing"></a>
|
||||
|
||||
## Routing
|
||||
|
||||
Non-draft pull requests run the workflow when opened, synchronized, reopened, or marked ready for review. 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.
|
||||
|
||||
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 `Changed code files`, `Excluded test files`, `Owners by changed file`, and `Reviewers to request` before it sends the review request. Unmatched files remain visible in the log. The pull-request author and users who are already requested are omitted.
|
||||
|
||||
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.
|
||||
|
||||
<a id="test-exclusion"></a>
|
||||
|
||||
## Test exclusion
|
||||
|
||||
Review routing excludes the repository's unit, end-to-end, expected-output, snapshot, benchmark, performance, stress, corpus, native, and Python test conventions. This includes `test`, `tests`, `__tests__`, `__snapshots__`, `benches`, and `stress-tests` directories; the top-level `benchmarks` and `snapshots` trees; `packages/test-support`; `scripts/fixtures` and `scripts/snapshots`; recognized test filename suffixes; and Python `test_*.py` or `*_test.py` files.
|
||||
|
||||
Test infrastructure that can alter how evidence is produced remains reviewable, including `vitest*.config.ts` and gate implementations under `scripts`. A production file named `test.ts`, `spec.ts`, or `snapshot.ts` is not excluded solely by that name.
|
||||
|
||||
<a id="security"></a>
|
||||
|
||||
## Security
|
||||
|
||||
The write-capable `pull_request_target` job checks out only the repository default branch. It does not check out or execute pull-request code and does not use repository secrets. Pull-request filenames are treated as API data and escaped in logs.
|
||||
|
||||
Ownership changes take effect only after they merge into the default branch. This prevents an untrusted pull request from changing the routing program or its owner assignments for its own run.
|
||||
|
||||
<a id="verification"></a>
|
||||
|
||||
## Verification
|
||||
|
||||
Run `pnpm run test:request-review` for ownership parsing, test classification, pagination, logging order, reviewer filtering, and API behavior. [Workflow tests](../../scripts/ci-workflow.spec.ts) pin the trusted checkout, permissions, events, and command. The repository gate graph runs both checks in CI.
|
||||
|
||||
<a id="dev-note"></a>
|
||||
|
||||
## Dev Note
|
||||
|
||||
The [review-routing decision](../../.agents/notes/implemented/process/2026-09-08-trusted-changed-file-review-routing.md) records the security model, test exclusions, and alternatives.
|
||||
@@ -0,0 +1,53 @@
|
||||
# 自动请求代码评审
|
||||
|
||||
[English](README.md) | 中文
|
||||
|
||||
## 概要
|
||||
|
||||
[`request-review` workflow](../workflows/request-review.yml) 从受信任的默认分支读取兼容 CODEOWNERS 格式的[所有权映射](CODEOWNERS)。它先打印完整的非测试变更文件列表,再将这些文件与 owner 匹配,最后请求尚未加入的评审者。所有权映射不在 GitHub 原生 CODEOWNERS 路径中,因此 GitHub 不会直接应用它。
|
||||
|
||||
## 目录
|
||||
|
||||
- [路由](#routing)
|
||||
- [排除测试](#test-exclusion)
|
||||
- [安全性](#security)
|
||||
- [验证](#verification)
|
||||
- [开发说明](#dev-note)
|
||||
|
||||
<a id="routing"></a>
|
||||
|
||||
## 路由
|
||||
|
||||
非草稿 PR 在创建、同步、重新打开或标记为可评审时运行该 workflow。扫描器获取完整的 PR 文件列表,分别检查重命名前后的路径;如果只能取得部分列表,则停止执行,不发出评审请求。GitHub 对此 API 最多公开 3,000 个文件。
|
||||
|
||||
所有权映射只接受显式绝对目录模式和 GitHub 个人用户。通配符、隐藏目录模式、团队,以及重复的模式或 owner 都会被拒绝。匹配遵循 CODEOWNERS 的最后一条匹配规则。扫描器先打印 `Changed code files`、`Excluded test files`、`Owners by changed file` 和 `Reviewers to request`,再发送评审请求。未匹配的文件仍显示在日志中。PR 作者和已经收到评审请求的用户会被排除。
|
||||
|
||||
策略测试会统计已匹配目录下的非测试跟踪文件行数,并要求 `@turtle1999` 拥有的有效代码库不超过三分之一。
|
||||
|
||||
<a id="test-exclusion"></a>
|
||||
|
||||
## 排除测试
|
||||
|
||||
评审路由会排除仓库中的单元测试、端到端测试、预期输出、快照、基准测试、性能测试、压力测试、语料测试、原生测试和 Python 测试约定。其中包括 `test`、`tests`、`__tests__`、`__snapshots__`、`benches` 和 `stress-tests` 目录,顶层 `benchmarks` 和 `snapshots` 目录树,`packages/test-support`、`scripts/fixtures` 和 `scripts/snapshots`,可识别的测试文件名后缀,以及 Python 的 `test_*.py` 或 `*_test.py` 文件。
|
||||
|
||||
能够改变证据生成方式的测试基础设施仍需评审,包括 `vitest*.config.ts` 和 `scripts` 下的门禁实现。生产文件不会仅因名称为 `test.ts`、`spec.ts` 或 `snapshot.ts` 而被排除。
|
||||
|
||||
<a id="security"></a>
|
||||
|
||||
## 安全性
|
||||
|
||||
具备写权限的 `pull_request_target` job 只检出仓库默认分支。它不会检出或执行 PR 代码,也不使用仓库 secret。PR 文件名仅作为 API 数据处理,并在日志中转义。
|
||||
|
||||
所有权变更只有合并到默认分支后才会生效。这可以防止不受信任的 PR 为自身的 workflow 运行修改路由程序或 owner 分配。
|
||||
|
||||
<a id="verification"></a>
|
||||
|
||||
## 验证
|
||||
|
||||
运行 `pnpm run test:request-review` 可检查所有权解析、测试分类、分页、日志顺序、评审者过滤和 API 行为。[Workflow 测试](../../scripts/ci-workflow.spec.ts)固定受信任检出、权限、事件和命令。仓库门禁图会在 CI 中运行这两类检查。
|
||||
|
||||
<a id="dev-note"></a>
|
||||
|
||||
## 开发说明
|
||||
|
||||
[评审路由决策](../../.agents/notes/implemented/process/2026-09-08-trusted-changed-file-review-routing.zh.md)记录了安全模型、测试排除规则和备选方案。
|
||||
@@ -0,0 +1,290 @@
|
||||
#!/usr/bin/env node
|
||||
|
||||
import { readFileSync } from 'node:fs'
|
||||
import process from 'node:process'
|
||||
import { pathToFileURL } from 'node:url'
|
||||
|
||||
const API_VERSION = '2026-03-10'
|
||||
const MAX_PULL_REQUEST_FILES = 3_000
|
||||
const PAGE_SIZE = 100
|
||||
const TEST_DIRECTORY_NAMES = new Set(['__snapshots__', '__tests__', 'benches', 'stress-tests', 'test', 'tests'])
|
||||
const TEST_FILE_MARKER = /\.(?:bench|corpus|e2e|perf|snapshot|spec|stress|test)\.[^./]+$/u
|
||||
const PYTHON_TEST_FILE = /^(?:test_.+|.+_tests?)\.py$/u
|
||||
|
||||
/**
|
||||
* Parse the explicit directory subset accepted from the review ownership file.
|
||||
* @param {string} source CODEOWNERS-compatible source text.
|
||||
* @returns {Array<{pattern: string, prefix: string, owners: string[]}>} Ordered ownership rules.
|
||||
*/
|
||||
export function parseOwnership(source) {
|
||||
const rules = []
|
||||
const patterns = new Set()
|
||||
for (const [index, rawLine] of source.split('\n').entries()) {
|
||||
const line = rawLine.trim()
|
||||
if (!line || line.startsWith('#')) continue
|
||||
const [pattern, ...owners] = line.split(/\s+/u)
|
||||
const location = `ownership line ${index + 1}`
|
||||
if (!/^\/[^*?[\]#!\\]+\/$/u.test(pattern)) {
|
||||
throw new Error(`${location}: expected one explicit absolute directory pattern`)
|
||||
}
|
||||
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`)
|
||||
const normalizedOwners = []
|
||||
const seenOwners = new Set()
|
||||
for (const owner of owners) {
|
||||
if (!/^@[A-Za-z0-9-]+$/u.test(owner)) {
|
||||
throw new Error(`${location}: only individual GitHub users are supported`)
|
||||
}
|
||||
const key = owner.toLowerCase()
|
||||
if (seenOwners.has(key)) throw new Error(`${location}: duplicate owner ${owner}`)
|
||||
seenOwners.add(key)
|
||||
normalizedOwners.push(owner)
|
||||
}
|
||||
patterns.add(pattern)
|
||||
rules.push({ pattern, prefix: pattern.slice(1), owners: normalizedOwners })
|
||||
}
|
||||
if (rules.length === 0) throw new Error('ownership file contains no rules')
|
||||
return rules
|
||||
}
|
||||
|
||||
/**
|
||||
* Normalize a repository-relative path received from GitHub.
|
||||
* @param {unknown} value GitHub file path.
|
||||
* @returns {string} Slash-normalized repository path.
|
||||
*/
|
||||
export function normalizeRepositoryPath(value) {
|
||||
if (typeof value !== 'string' || value.length === 0) throw new Error('changed file has no path')
|
||||
const normalized = value.replaceAll('\\', '/').replace(/^\.\/+/, '')
|
||||
if (
|
||||
normalized.startsWith('/')
|
||||
|| normalized.includes('\0')
|
||||
|| normalized.split('/').some(segment => !segment || segment === '.' || segment === '..')
|
||||
) {
|
||||
throw new Error(`invalid repository path ${JSON.stringify(value)}`)
|
||||
}
|
||||
return normalized
|
||||
}
|
||||
|
||||
/**
|
||||
* Decide whether a repository path belongs only to test evidence or test support.
|
||||
* @param {string} value Repository-relative path.
|
||||
* @returns {boolean} Whether reviewer routing must ignore the path.
|
||||
*/
|
||||
export function isTestPath(value) {
|
||||
const file = normalizeRepositoryPath(value)
|
||||
const segments = file.split('/')
|
||||
if (segments[0] === 'benchmarks' || segments[0] === 'snapshots') return true
|
||||
if (segments[0] === 'packages' && segments[1] === 'test-support') return true
|
||||
if (segments[0] === 'scripts' && (segments[1] === 'fixtures' || segments[1] === 'snapshots')) return true
|
||||
if (segments.some(segment => TEST_DIRECTORY_NAMES.has(segment))) return true
|
||||
const basename = segments.at(-1) ?? ''
|
||||
return TEST_FILE_MARKER.test(basename) || PYTHON_TEST_FILE.test(basename)
|
||||
}
|
||||
|
||||
/**
|
||||
* Expand changed-file records into reviewable and excluded repository paths.
|
||||
* @param {unknown[]} files Pull-request file records from GitHub.
|
||||
* @returns {{changedCodeFiles: string[], excludedTestFiles: string[]}} Classified paths.
|
||||
*/
|
||||
export function classifyChangedFiles(files) {
|
||||
const changedCodeFiles = new Set()
|
||||
const excludedTestFiles = new Set()
|
||||
for (const entry of files) {
|
||||
if (!isRecord(entry)) throw new Error('changed-file response contains a non-object entry')
|
||||
const paths = [normalizeRepositoryPath(entry.filename)]
|
||||
if (entry.previous_filename !== undefined) {
|
||||
paths.unshift(normalizeRepositoryPath(entry.previous_filename))
|
||||
}
|
||||
for (const file of paths) {
|
||||
if (isTestPath(file)) excludedTestFiles.add(file)
|
||||
else changedCodeFiles.add(file)
|
||||
}
|
||||
}
|
||||
return {
|
||||
changedCodeFiles: [...changedCodeFiles].sort(),
|
||||
excludedTestFiles: [...excludedTestFiles].sort(),
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Match changed paths to owners with CODEOWNERS last-match semantics.
|
||||
* @param {Array<{prefix: string, owners: string[]}>} rules Ordered ownership rules.
|
||||
* @param {string[]} changedCodeFiles Reviewable repository paths.
|
||||
* @returns {{matches: Array<{file: string, owners: string[]}>, reviewers: string[]}} Routing plan.
|
||||
*/
|
||||
export function planReviewers(rules, changedCodeFiles) {
|
||||
const matches = []
|
||||
const reviewers = new Map()
|
||||
for (const file of changedCodeFiles) {
|
||||
let owners = []
|
||||
for (const rule of rules) {
|
||||
if (file.startsWith(rule.prefix)) owners = rule.owners
|
||||
}
|
||||
matches.push({ file, owners })
|
||||
for (const owner of owners) reviewers.set(owner.toLowerCase(), owner.slice(1))
|
||||
}
|
||||
return {
|
||||
matches,
|
||||
reviewers: [...reviewers.values()].sort((left, right) => left.localeCompare(right, 'en')),
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a repository-scoped GitHub JSON API caller.
|
||||
* @param {{token: string, apiUrl?: string, fetchImpl?: typeof fetch}} options API dependencies.
|
||||
* @returns {(path: string, options?: {method?: string, body?: unknown}) => Promise<unknown>} API caller.
|
||||
*/
|
||||
export function createGitHubApi({ token, apiUrl = 'https://api.github.com', fetchImpl = globalThis.fetch }) {
|
||||
if (!token) throw new Error('GITHUB_TOKEN is not set')
|
||||
if (typeof fetchImpl !== 'function') throw new Error('fetch is unavailable')
|
||||
const root = apiUrl.replace(/\/+$/u, '')
|
||||
return async (path, { method = 'GET', body } = {}) => {
|
||||
const response = await fetchImpl(`${root}${path}`, {
|
||||
method,
|
||||
headers: {
|
||||
Accept: 'application/vnd.github+json',
|
||||
Authorization: `Bearer ${token}`,
|
||||
'Content-Type': 'application/json',
|
||||
'User-Agent': 'deepseek-harness-request-review',
|
||||
'X-GitHub-Api-Version': API_VERSION,
|
||||
},
|
||||
...(body === undefined ? {} : { body: JSON.stringify(body) }),
|
||||
})
|
||||
if (!response.ok) {
|
||||
const responseBody = await response.text()
|
||||
throw new Error(`GitHub API ${method} ${path} returned ${response.status}: ${JSON.stringify(responseBody)}`)
|
||||
}
|
||||
if (response.status === 204) return undefined
|
||||
return response.json()
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetch the complete pull-request file list or fail before routing a partial list.
|
||||
* @param {(path: string, options?: {method?: string, body?: unknown}) => Promise<unknown>} api GitHub API caller.
|
||||
* @param {string} repository Owner/name repository identifier.
|
||||
* @param {number} pullNumber Pull-request number.
|
||||
* @param {number} expectedCount Pull-request changed-file count.
|
||||
* @returns {Promise<unknown[]>} Complete changed-file records.
|
||||
*/
|
||||
export async function listPullRequestFiles(api, repository, pullNumber, expectedCount) {
|
||||
if (!Number.isSafeInteger(expectedCount) || expectedCount < 0) {
|
||||
throw new Error('pull request changed_files must be a non-negative integer')
|
||||
}
|
||||
if (expectedCount > MAX_PULL_REQUEST_FILES) {
|
||||
throw new Error(`pull request has ${expectedCount} files; GitHub exposes at most ${MAX_PULL_REQUEST_FILES}`)
|
||||
}
|
||||
const files = []
|
||||
for (let page = 1; files.length < expectedCount; page++) {
|
||||
const response = await api(`/repos/${repository}/pulls/${pullNumber}/files?per_page=${PAGE_SIZE}&page=${page}`)
|
||||
if (!Array.isArray(response) || response.length === 0) {
|
||||
throw new Error(`GitHub returned ${files.length} of ${expectedCount} changed files`)
|
||||
}
|
||||
files.push(...response)
|
||||
if (files.length > expectedCount) {
|
||||
throw new Error(`GitHub returned ${files.length} files but the pull request reports ${expectedCount}`)
|
||||
}
|
||||
}
|
||||
return files
|
||||
}
|
||||
|
||||
/**
|
||||
* Print changed paths, route owners, and request every missing eligible reviewer.
|
||||
* @param {{event: unknown, ownershipSource: string, api: (path: string, options?: {method?: string, body?: unknown}) => Promise<unknown>, write?: (line: string) => void}} options Runtime inputs.
|
||||
* @returns {Promise<{changedCodeFiles: string[], excludedTestFiles: string[], requestedReviewers: string[]}>} Applied routing result.
|
||||
*/
|
||||
export async function requestReviews({ event, ownershipSource, api, write = line => process.stdout.write(`${line}\n`) }) {
|
||||
const pull = pullRequestFromEvent(event)
|
||||
write('This is by automated Angry Turtle Cyborg, not a human')
|
||||
if (pull.draft) {
|
||||
write('Draft pull request; reviewer routing is deferred until ready_for_review.')
|
||||
return { changedCodeFiles: [], excludedTestFiles: [], requestedReviewers: [] }
|
||||
}
|
||||
|
||||
const files = await listPullRequestFiles(api, pull.repository, pull.number, pull.changedFileCount)
|
||||
const classified = classifyChangedFiles(files)
|
||||
const plan = planReviewers(parseOwnership(ownershipSource), classified.changedCodeFiles)
|
||||
writeList(write, 'Changed code files', classified.changedCodeFiles.map(file => JSON.stringify(file)))
|
||||
writeList(write, 'Excluded test files', classified.excludedTestFiles.map(file => JSON.stringify(file)))
|
||||
writeList(
|
||||
write,
|
||||
'Owners by changed file',
|
||||
plan.matches.map(({ file, owners }) => `${JSON.stringify(file)}: ${owners.length ? owners.join(' ') : '(none)'}`),
|
||||
)
|
||||
|
||||
const candidates = plan.reviewers.filter(login => login.toLowerCase() !== pull.author.toLowerCase())
|
||||
if (candidates.length === 0) {
|
||||
writeList(write, 'Reviewers to request', [])
|
||||
return { ...classified, requestedReviewers: [] }
|
||||
}
|
||||
const existing = await api(`/repos/${pull.repository}/pulls/${pull.number}/requested_reviewers`)
|
||||
if (!isRecord(existing) || !Array.isArray(existing.users)) {
|
||||
throw new Error('requested-reviewers response has no users array')
|
||||
}
|
||||
const alreadyRequested = new Set(existing.users.map((user) => {
|
||||
if (!isRecord(user) || typeof user.login !== 'string') {
|
||||
throw new Error('requested-reviewers response contains an invalid user')
|
||||
}
|
||||
return user.login.toLowerCase()
|
||||
}))
|
||||
const reviewers = candidates.filter(login => !alreadyRequested.has(login.toLowerCase()))
|
||||
writeList(write, 'Reviewers to request', reviewers.map(login => `@${login}`))
|
||||
if (reviewers.length === 0) return { ...classified, requestedReviewers: [] }
|
||||
|
||||
await api(`/repos/${pull.repository}/pulls/${pull.number}/requested_reviewers`, {
|
||||
method: 'POST',
|
||||
body: { reviewers },
|
||||
})
|
||||
write(`Requested ${reviewers.map(login => `@${login}`).join(' ')}.`)
|
||||
return { ...classified, requestedReviewers: reviewers }
|
||||
}
|
||||
|
||||
function pullRequestFromEvent(event) {
|
||||
if (!isRecord(event) || !isRecord(event.repository) || typeof event.repository.full_name !== 'string') {
|
||||
throw new Error('event has no repository.full_name')
|
||||
}
|
||||
if (!isRecord(event.pull_request) || !isRecord(event.pull_request.user)) {
|
||||
throw new Error('event has no pull_request')
|
||||
}
|
||||
const { pull_request: pull } = event
|
||||
if (!Number.isSafeInteger(pull.number) || pull.number <= 0) throw new Error('pull request has no valid number')
|
||||
if (typeof pull.draft !== 'boolean') throw new Error('pull request has no draft flag')
|
||||
if (typeof pull.user.login !== 'string' || !pull.user.login) throw new Error('pull request has no author login')
|
||||
return {
|
||||
repository: event.repository.full_name,
|
||||
number: pull.number,
|
||||
draft: pull.draft,
|
||||
author: pull.user.login,
|
||||
changedFileCount: pull.changed_files,
|
||||
}
|
||||
}
|
||||
|
||||
function writeList(write, title, entries) {
|
||||
write(`${title}:`)
|
||||
if (entries.length === 0) write('- (none)')
|
||||
else for (const entry of entries) write(`- ${entry}`)
|
||||
}
|
||||
|
||||
function isRecord(value) {
|
||||
return typeof value === 'object' && value !== null && !Array.isArray(value)
|
||||
}
|
||||
|
||||
async function main() {
|
||||
const eventPath = process.env.GITHUB_EVENT_PATH
|
||||
if (!eventPath) throw new Error('GITHUB_EVENT_PATH is not set')
|
||||
const event = JSON.parse(readFileSync(eventPath, 'utf8'))
|
||||
const ownershipSource = readFileSync(new URL('CODEOWNERS', import.meta.url), 'utf8')
|
||||
const api = createGitHubApi({
|
||||
token: process.env.GITHUB_TOKEN ?? '',
|
||||
apiUrl: process.env.GITHUB_API_URL,
|
||||
})
|
||||
await requestReviews({ event, ownershipSource, api })
|
||||
}
|
||||
|
||||
if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {
|
||||
main().catch((error) => {
|
||||
process.stderr.write(`request-review failed: ${error instanceof Error ? error.message : String(error)}\n`)
|
||||
process.exitCode = 1
|
||||
})
|
||||
}
|
||||
@@ -0,0 +1,335 @@
|
||||
import assert from 'node:assert/strict'
|
||||
import { execFileSync } from 'node:child_process'
|
||||
import { readFileSync } from 'node:fs'
|
||||
import test from 'node:test'
|
||||
|
||||
import {
|
||||
classifyChangedFiles,
|
||||
createGitHubApi,
|
||||
isTestPath,
|
||||
listPullRequestFiles,
|
||||
normalizeRepositoryPath,
|
||||
parseOwnership,
|
||||
planReviewers,
|
||||
requestReviews,
|
||||
} from './request-review.mjs'
|
||||
|
||||
const ownershipSource = readFileSync(new URL('CODEOWNERS', import.meta.url), 'utf8')
|
||||
|
||||
const pullRequestEvent = ({ author = 'author', changedFiles = 1, draft = false } = {}) => ({
|
||||
repository: { full_name: 'deepseek-harness/deepseek-harness' },
|
||||
pull_request: {
|
||||
number: 42,
|
||||
draft,
|
||||
changed_files: changedFiles,
|
||||
user: { login: author },
|
||||
},
|
||||
})
|
||||
|
||||
test('loads the repository ownership policy without test-only directory rules', () => {
|
||||
const rules = parseOwnership(ownershipSource)
|
||||
const ownersByPattern = new Map(rules.map(rule => [rule.pattern, rule.owners]))
|
||||
assert.equal(rules.length, 58)
|
||||
assert.equal(rules.some(rule => rule.pattern === '/benchmarks/'), false)
|
||||
assert.equal(rules.some(rule => rule.pattern === '/snapshots/'), false)
|
||||
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/llm/'), ['@LegGasai'])
|
||||
assert.deepEqual(ownersByPattern.get('/packages/preset/'), ['@LegGasai', '@turtle1999'])
|
||||
assert.deepEqual(ownersByPattern.get('/packages/session/'), ['@tianyicui', '@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.some(rule => rule.owners.some(owner => owner.toLowerCase() === excludedOwner)), false)
|
||||
}
|
||||
})
|
||||
|
||||
test('keeps turtle below one third of the eligible owned codebase', () => {
|
||||
const rules = parseOwnership(ownershipSource)
|
||||
const trackedFiles = execFileSync('git', ['ls-files', '-z'], { encoding: 'utf8' }).split('\0').filter(Boolean)
|
||||
let ownedLines = 0
|
||||
let turtleLines = 0
|
||||
for (const file of trackedFiles) {
|
||||
if (isTestPath(file)) continue
|
||||
const owners = planReviewers(rules, [file]).matches[0]?.owners ?? []
|
||||
if (owners.length === 0) continue
|
||||
const content = readFileSync(file)
|
||||
const lines = content.length === 0
|
||||
? 0
|
||||
: content.reduce((count, byte) => count + (byte === 10 ? 1 : 0), 0) + (content.at(-1) === 10 ? 0 : 1)
|
||||
ownedLines += lines
|
||||
if (owners.includes('@turtle1999')) turtleLines += lines
|
||||
}
|
||||
assert.ok(
|
||||
turtleLines * 3 <= ownedLines,
|
||||
`@turtle1999 owns ${turtleLines} of ${ownedLines} eligible owned lines`,
|
||||
)
|
||||
})
|
||||
|
||||
test('rejects ownership forms the requester cannot apply safely', () => {
|
||||
for (const [source, message] of [
|
||||
['', /contains no rules/u],
|
||||
['* @owner\n', /explicit absolute directory/u],
|
||||
['/.github/ @owner\n', /hidden-directory/u],
|
||||
['/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/ @owner @OWNER\n', /duplicate owner/u],
|
||||
['/packages/core/ @owner\n/packages/core/ @other\n', /duplicate pattern/u],
|
||||
]) {
|
||||
assert.throws(() => parseOwnership(source), message)
|
||||
}
|
||||
})
|
||||
|
||||
test('recognizes every repository test location and filename convention', () => {
|
||||
for (const file of [
|
||||
'apps/cli/tests/args.spec.ts',
|
||||
'apps/cli/tests/harness.ts',
|
||||
'apps/web/stress-tests/reasoning-chunks.stress.ts',
|
||||
'benchmarks/session-open/workload.ts',
|
||||
'native/landlock-run/test/entry.test.js',
|
||||
'packages/core/agent/__tests__/agent.ts',
|
||||
'packages/core/agent/benches/agent.rs',
|
||||
'packages/core/agent/src/agent.compat.spec.ts',
|
||||
'packages/core/agent/src/__snapshots__/agent.ts.snap',
|
||||
'packages/session-query/session-query/tests/test-service.ts',
|
||||
'packages/test-support/session-snapshot/src/index.ts',
|
||||
'python/sdk/src/test_client.py',
|
||||
'python/sdk/src/client_test.py',
|
||||
'scripts/fixtures/translation-prompt/response.txt',
|
||||
'scripts/session-snapshot-corpus.corpus.ts',
|
||||
'scripts/snapshots/translation-prompt-v4/request-response.expected.json',
|
||||
'snapshots/session/headless.snapshot.ts',
|
||||
]) {
|
||||
assert.equal(isTestPath(file), true, file)
|
||||
}
|
||||
})
|
||||
|
||||
test('does not confuse production names with tests', () => {
|
||||
for (const file of [
|
||||
'apps/cli/src/testing.ts',
|
||||
'packages/core/agent/src/contest.ts',
|
||||
'packages/session/session-format/src/snapshot.ts',
|
||||
'packages/session/session-format/src/spec.ts',
|
||||
'packages/session/session-format/src/test.ts',
|
||||
'scripts/run-gates.ts',
|
||||
'vitest.config.ts',
|
||||
'vitest.bench.config.ts',
|
||||
'vitest.e2e.config.ts',
|
||||
'vitest.snapshot.config.ts',
|
||||
'vitest.web.perf.config.ts',
|
||||
'website/docs.ts',
|
||||
]) {
|
||||
assert.equal(isTestPath(file), false, file)
|
||||
}
|
||||
})
|
||||
|
||||
test('normalizes separators and rejects paths that are not repository-relative', () => {
|
||||
assert.equal(normalizeRepositoryPath('./packages\\core\\agent\\src\\index.ts'), 'packages/core/agent/src/index.ts')
|
||||
for (const file of ['', '/absolute.ts', '../escape.ts', 'packages//empty.ts', 'packages/./same.ts']) {
|
||||
assert.throws(() => normalizeRepositoryPath(file), /path/u, file)
|
||||
}
|
||||
})
|
||||
|
||||
test('classifies both sides of a rename independently', () => {
|
||||
assert.deepEqual(
|
||||
classifyChangedFiles([
|
||||
{
|
||||
filename: 'packages/core/agent/tests/moved.spec.ts',
|
||||
previous_filename: 'packages/core/agent/src/moved.ts',
|
||||
},
|
||||
{
|
||||
filename: 'packages/client/store/src/restored.ts',
|
||||
previous_filename: 'packages/client/store/tests/restored.spec.ts',
|
||||
},
|
||||
]),
|
||||
{
|
||||
changedCodeFiles: [
|
||||
'packages/client/store/src/restored.ts',
|
||||
'packages/core/agent/src/moved.ts',
|
||||
],
|
||||
excludedTestFiles: [
|
||||
'packages/client/store/tests/restored.spec.ts',
|
||||
'packages/core/agent/tests/moved.spec.ts',
|
||||
],
|
||||
},
|
||||
)
|
||||
})
|
||||
|
||||
test('uses the last matching ownership rule and keeps unmatched files visible', () => {
|
||||
const rules = parseOwnership('/packages/ @broad\n/packages/core/ @core @second\n')
|
||||
assert.deepEqual(
|
||||
planReviewers(rules, ['AGENTS.md', 'packages/core/agent/src/index.ts', 'packages/fs/fs/src/index.ts']),
|
||||
{
|
||||
matches: [
|
||||
{ file: 'AGENTS.md', owners: [] },
|
||||
{ file: 'packages/core/agent/src/index.ts', owners: ['@core', '@second'] },
|
||||
{ file: 'packages/fs/fs/src/index.ts', owners: ['@broad'] },
|
||||
],
|
||||
reviewers: ['broad', 'core', 'second'],
|
||||
},
|
||||
)
|
||||
})
|
||||
|
||||
test('fetches every declared changed file across pages', async () => {
|
||||
const calls = []
|
||||
const pageOne = Array.from({ length: 100 }, (_, index) => ({ filename: `packages/core/file-${index}.ts` }))
|
||||
const pageTwo = [{ filename: 'packages/core/file-100.ts' }]
|
||||
const api = async (path) => {
|
||||
calls.push(path)
|
||||
return calls.length === 1 ? pageOne : pageTwo
|
||||
}
|
||||
const files = await listPullRequestFiles(api, 'owner/repo', 42, 101)
|
||||
assert.equal(files.length, 101)
|
||||
assert.deepEqual(calls, [
|
||||
'/repos/owner/repo/pulls/42/files?per_page=100&page=1',
|
||||
'/repos/owner/repo/pulls/42/files?per_page=100&page=2',
|
||||
])
|
||||
})
|
||||
|
||||
test('fails closed when GitHub cannot provide the complete file list', async () => {
|
||||
let calls = 0
|
||||
await assert.rejects(
|
||||
listPullRequestFiles(async () => {
|
||||
calls++
|
||||
return calls === 1 ? [{ filename: 'one.ts' }] : []
|
||||
}, 'owner/repo', 42, 2),
|
||||
/returned 1 of 2/u,
|
||||
)
|
||||
await assert.rejects(
|
||||
listPullRequestFiles(async () => [], 'owner/repo', 42, 3_001),
|
||||
/at most 3000/u,
|
||||
)
|
||||
})
|
||||
|
||||
test('prints changed code files before requesting missing owners', async () => {
|
||||
const trace = []
|
||||
const files = [
|
||||
{ filename: 'packages/core/agent/src/index.ts' },
|
||||
{ filename: 'packages/preset/agent-presets/src/index.ts' },
|
||||
{ filename: 'packages/client/store/src/index.ts' },
|
||||
{ filename: 'packages/subagent/subagent/src/index.ts' },
|
||||
{ filename: 'packages/core/agent/tests/index.spec.ts' },
|
||||
{ filename: 'AGENTS.md' },
|
||||
]
|
||||
const api = async (path, options = {}) => {
|
||||
trace.push({ type: 'api', path, options })
|
||||
if (path.endsWith('/files?per_page=100&page=1')) return files
|
||||
if (path.endsWith('/requested_reviewers') && options.method !== 'POST') {
|
||||
return { users: [{ login: 'imccyu' }], teams: [] }
|
||||
}
|
||||
if (path.endsWith('/requested_reviewers') && options.method === 'POST') return {}
|
||||
throw new Error(`unexpected API path ${path}`)
|
||||
}
|
||||
|
||||
const result = await requestReviews({
|
||||
event: pullRequestEvent({ author: 'turtle1999', changedFiles: files.length }),
|
||||
ownershipSource,
|
||||
api,
|
||||
write: line => trace.push({ type: 'log', line }),
|
||||
})
|
||||
|
||||
assert.deepEqual(result, {
|
||||
changedCodeFiles: [
|
||||
'AGENTS.md',
|
||||
'packages/client/store/src/index.ts',
|
||||
'packages/core/agent/src/index.ts',
|
||||
'packages/preset/agent-presets/src/index.ts',
|
||||
'packages/subagent/subagent/src/index.ts',
|
||||
],
|
||||
excludedTestFiles: ['packages/core/agent/tests/index.spec.ts'],
|
||||
requestedReviewers: ['Dudu-0223', 'LegGasai', 'mektpoy', 'tianyicui'],
|
||||
})
|
||||
assert.equal(trace[0].type, 'log')
|
||||
assert.equal(trace[0].line, 'This is by automated Angry Turtle Cyborg, not a human')
|
||||
const changedHeading = trace.findIndex(item => item.type === 'log' && item.line === 'Changed code files:')
|
||||
const post = trace.findIndex(item => item.type === 'api' && item.options.method === 'POST')
|
||||
assert.ok(changedHeading >= 0 && changedHeading < post)
|
||||
assert.deepEqual(trace[post], {
|
||||
type: 'api',
|
||||
path: '/repos/deepseek-harness/deepseek-harness/pulls/42/requested_reviewers',
|
||||
options: {
|
||||
method: 'POST',
|
||||
body: { reviewers: ['Dudu-0223', 'LegGasai', 'mektpoy', 'tianyicui'] },
|
||||
},
|
||||
})
|
||||
})
|
||||
|
||||
test('does not request reviewers for a test-only change', async () => {
|
||||
const calls = []
|
||||
const output = []
|
||||
const files = [
|
||||
{ filename: 'apps/web/tests/chat.e2e.ts' },
|
||||
{ filename: 'packages/core/agent/tests/agent.spec.ts' },
|
||||
]
|
||||
const result = await requestReviews({
|
||||
event: pullRequestEvent({ changedFiles: files.length }),
|
||||
ownershipSource,
|
||||
api: async (path) => {
|
||||
calls.push(path)
|
||||
return files
|
||||
},
|
||||
write: line => output.push(line),
|
||||
})
|
||||
assert.deepEqual(result, {
|
||||
changedCodeFiles: [],
|
||||
excludedTestFiles: files.map(file => file.filename),
|
||||
requestedReviewers: [],
|
||||
})
|
||||
assert.equal(calls.length, 1)
|
||||
assert.deepEqual(output.slice(0, 4), [
|
||||
'This is by automated Angry Turtle Cyborg, not a human',
|
||||
'Changed code files:',
|
||||
'- (none)',
|
||||
'Excluded test files:',
|
||||
])
|
||||
})
|
||||
|
||||
test('defers draft pull requests without reading changed files', async () => {
|
||||
const output = []
|
||||
const result = await requestReviews({
|
||||
event: pullRequestEvent({ draft: true }),
|
||||
ownershipSource,
|
||||
api: async () => assert.fail('draft routing must not call GitHub'),
|
||||
write: line => output.push(line),
|
||||
})
|
||||
assert.deepEqual(result, { changedCodeFiles: [], excludedTestFiles: [], requestedReviewers: [] })
|
||||
assert.deepEqual(output, [
|
||||
'This is by automated Angry Turtle Cyborg, not a human',
|
||||
'Draft pull request; reviewer routing is deferred until ready_for_review.',
|
||||
])
|
||||
})
|
||||
|
||||
test('sends authenticated JSON and escapes an API error body', async () => {
|
||||
const requests = []
|
||||
const api = createGitHubApi({
|
||||
token: 'secret',
|
||||
apiUrl: 'https://github.example/api/v3/',
|
||||
fetchImpl: async (url, options) => {
|
||||
requests.push({ url, options })
|
||||
return new Response(JSON.stringify({ ok: true }), {
|
||||
status: 200,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
})
|
||||
},
|
||||
})
|
||||
assert.deepEqual(await api('/repos/owner/repo', { method: 'POST', body: { value: 1 } }), { ok: true })
|
||||
assert.equal(requests[0].url, 'https://github.example/api/v3/repos/owner/repo')
|
||||
assert.equal(requests[0].options.headers.Authorization, 'Bearer secret')
|
||||
assert.equal(requests[0].options.headers['X-GitHub-Api-Version'], '2026-03-10')
|
||||
assert.equal(requests[0].options.body, '{"value":1}')
|
||||
|
||||
const failing = createGitHubApi({
|
||||
token: 'secret',
|
||||
fetchImpl: async () => new Response('::error::untrusted\nbody', { status: 422 }),
|
||||
})
|
||||
await assert.rejects(failing('/failure'), /"::error::untrusted\\nbody"/u)
|
||||
})
|
||||
@@ -0,0 +1,32 @@
|
||||
name: request-review
|
||||
|
||||
on:
|
||||
pull_request_target:
|
||||
types: [opened, synchronize, reopened, ready_for_review]
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
pull-requests: write
|
||||
|
||||
concurrency:
|
||||
group: request-review-${{ github.event.pull_request.number }}
|
||||
cancel-in-progress: true
|
||||
|
||||
jobs:
|
||||
request-review:
|
||||
name: request-review
|
||||
if: ${{ !github.event.pull_request.draft }}
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 5
|
||||
steps:
|
||||
# SECURITY: the write-capable job executes policy from the trusted default
|
||||
# branch and reads pull-request filenames only as API data.
|
||||
- name: Check out trusted review policy
|
||||
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1
|
||||
with:
|
||||
ref: ${{ github.event.repository.default_branch }}
|
||||
persist-credentials: false
|
||||
- name: Request reviewers
|
||||
env:
|
||||
GITHUB_TOKEN: ${{ github.token }}
|
||||
run: node .github/review-ownership/request-review.mjs
|
||||
Reference in New Issue
Block a user