diff --git a/README.md b/README.md index 407c479..c870155 100644 --- a/README.md +++ b/README.md @@ -227,8 +227,22 @@ feature/xxx ──PR──> test ──PR──> prd ↑ 第二级:同样审查通过才合并 ``` +**直接推送**也走同一条链路:往 `test` 推代码(没有 PR)时,审查通过后服务会 +自动开一个 `test → prd` 的 PR 并合并它,然后关闭该仓库所有审查 Issue。 + +自动开的晋级 PR 会带一个标记,它自身的 webhook 事件会被忽略——同一批提交在 +push 时已经审过,重复审会在代码合并后凭空造出 Issue。 + +晋级 PR 始终使用 `merge` 而非 `squash`:squash 会把提交重写成全新提交, +两个长期共存的分支每晋级一次就多分叉一点,最终必然冲突;真正的 merge +保留共同祖先,下次晋级只携带新提交。 + `managed_branch` 只决定「push 事件没有对应 PR 时,拿哪个分支做对比基准」,以及新仓库的默认值。 +**Issue 生命周期**:只要发生合并(服务自动合并、或人在 Gitea 里手动合并), +该仓库所有由本服务创建的 open Issue 都会被评论并关闭——代码已经落地, +上一轮的问题描述的是不再存在的状态。下一个问题会在下一次审查时重新开 Issue。 + **审查范围**:PR 事件优先用 PR 的目标分支作为基准;push 事件优先用 webhook 里的 `before`,当 `before` 是新建分支的全零值或缺失时,退回到与管理分支的 merge-base。 **审查背景**:仓库配置里的「审查背景 / 需求」是多行文本,既作为 OCR 的 `--background` 传给模型,也会完整记录到审查历史,方便回溯「当时是按什么需求审的」。 diff --git a/app/lib/gitea.js b/app/lib/gitea.js index 21a24bb..b06bdc9 100644 --- a/app/lib/gitea.js +++ b/app/lib/gitea.js @@ -232,6 +232,27 @@ export class GiteaClient { return this.get(`/api/v1/repos/${owner}/${repo}/pulls/${index}`); } + /** Open a pull request. */ + async createPullRequest(owner, repo, { title, head, base, body = "" }) { + return this.post(`/api/v1/repos/${owner}/${repo}/pulls`, { title, head, base, body }); + } + + /** Close a pull request without merging it. */ + async closePullRequest(owner, repo, index) { + return this.patch(`/api/v1/repos/${owner}/${repo}/pulls/${index}`, { state: "closed" }); + } + + /** Compare two refs; returns the file list, or null when they are identical. */ + async compareRefs(owner, repo, base, head) { + try { + return await this.get(`/api/v1/repos/${owner}/${repo}/compare/${base}...${head}`); + } catch (err) { + // Gitea answers 404 when the two refs point at the same commit. + if (err.status === 404) return null; + throw err; + } + } + async listPullRequests(owner, repo, query = {}) { return this.get(`/api/v1/repos/${owner}/${repo}/pulls`, { query }); } diff --git a/app/lib/ocr.js b/app/lib/ocr.js index f562ae2..f1399ac 100644 --- a/app/lib/ocr.js +++ b/app/lib/ocr.js @@ -183,6 +183,14 @@ export class OcrRunner { return res.stdout.trim().split("\n")[0]; } + /** Parent commit count for a commit; >1 means it is a merge commit. */ + async parentCount(dir, sha) { + const res = await git(["rev-list", "--parents", "-n", "1", sha], { cwd: dir, timeoutMs: 60000 }); + if (res.code !== 0) return 0; + const parts = res.stdout.trim().split(/\s+/).filter(Boolean); + return Math.max(0, parts.length - 1); + } + async changedFiles(dir, fromSha, toSha) { const res = await git( ["diff", "--name-only", "--diff-filter=ACMRTUXB", fromSha, toSha], diff --git a/app/lib/review.js b/app/lib/review.js index 3449f45..ea85e9d 100644 --- a/app/lib/review.js +++ b/app/lib/review.js @@ -14,6 +14,9 @@ import { export const SUMMARY_MARKER = ""; export const COMMENT_MARKER = ""; +// Marks a pull request this service opened itself to promote a branch. Its +// webhook event is ignored because the same commits were already reviewed. +export const PROMOTION_MARKER = ""; export const STATUS_CONTEXT = "code-review/ocr"; export class SkipJob extends Error { @@ -645,14 +648,20 @@ export class ReviewEngine { return created?.number ?? null; } + /** + * Get the change into the next branch up the chain. + * + * When the reviewed ref has an open pull request we merge it. When it does + * not (a direct push to a checked branch) we open one automatically, so a + * push to `test` still flows into `prd` with a reviewable record. + */ async maybeAutoMerge({ client, repo, job, prNumber, blocking, toSha, appendLog, reviewIncomplete = false, statusState = "success", }) { if (!repo.auto_merge) return false; if (!prNumber) { - appendLog("auto-merge skipped: no pull request for this branch"); - return false; + return this.promoteBranch({ client, repo, job, toSha, appendLog, reviewIncomplete, statusState }); } // A two-stage flow (feature -> test -> prd) legitimately merges into a // checked branch that is not the managed branch, so the gate is "is the @@ -706,7 +715,8 @@ export class ReviewEngine { headCommitId: toSha, mergeWhenChecksSucceed: repo.auto_merge_mode === "when_checks_succeed", }); - appendLog("auto-merge requested"); + appendLog(`auto-merge requested for PR #${prNumber}`); + await this.closeRepoIssues({ client, repo, appendLog, reason: `PR #${prNumber} 已合并` }); return true; } catch (err) { appendLog(`auto-merge failed: ${err.message}`); @@ -714,6 +724,192 @@ export class ReviewEngine { } } + /** + * Open a pull request from the pushed branch into the next branch in the + * chain and merge it, then close the repository's review issues. + */ + async promoteBranch({ client, repo, job, toSha, appendLog, reviewIncomplete, statusState }) { + if (reviewIncomplete) { + appendLog("auto-merge skipped: review coverage was incomplete"); + return false; + } + if (statusState !== "success") { + appendLog(`auto-merge skipped: commit status is ${statusState}`); + return false; + } + if (!branchMatches(job.ref_name, repo.check_branches)) { + appendLog(`auto-merge skipped: ${job.ref_name} is not a checked branch`); + return false; + } + + // Walk up the chain: test -> prd. The managed branch is the top, so a push + // to it has nowhere to go. + const managed = repo.managed_branch; + if (job.ref_name === managed) { + appendLog(`auto-merge skipped: ${job.ref_name} is the managed branch`); + return false; + } + const target = managed; + if (!branchMatches(target, repo.check_branches) && target !== managed) { + appendLog(`auto-merge skipped: ${target} is not a checked branch`); + return false; + } + + let head; + let base; + try { + head = await client.getBranch(repo.owner, repo.name, job.ref_name); + base = await client.getBranch(repo.owner, repo.name, target); + } catch (err) { + appendLog(`auto-merge skipped: cannot resolve branches: ${err.message}`); + return false; + } + const headSha = head?.commit?.id; + const baseSha = base?.commit?.id; + if (!headSha || !baseSha) { + appendLog("auto-merge skipped: branch head unknown"); + return false; + } + if (headSha === baseSha) { + appendLog(`auto-merge skipped: ${job.ref_name} already matches ${target}`); + return false; + } + // Only promote the exact commit that was reviewed. + if (headSha !== toSha) { + appendLog(`auto-merge skipped: ${job.ref_name} moved to ${headSha.slice(0, 10)} after the review`); + return false; + } + + // Is there already an open PR for this promotion? Reuse it instead of + // opening a duplicate on every push. + let promotionPr = null; + try { + const open = await client.listPullRequests(repo.owner, repo.name, { state: "open", limit: 50 }); + promotionPr = (open || []).find( + (p) => p.head?.ref === job.ref_name && p.base?.ref === target, + ) ?? null; + } catch { /* fall through to creating one */ } + + if (!promotionPr) { + try { + promotionPr = await client.createPullRequest(repo.owner, repo.name, { + title: `[OCR] ${job.ref_name} → ${target}`, + head: job.ref_name, + base: target, + body: [ + PROMOTION_MARKER, + `由 gitea-codereview 自动创建。`, + "", + `- 来源分支:\`${job.ref_name}\``, + `- 目标分支:\`${target}\``, + `- 审查提交:\`${toSha}\``, + `- 代码审查已通过(job #${job.id})`, + ].join("\n"), + }); + appendLog(`opened promotion PR #${promotionPr?.number} (${job.ref_name} -> ${target})`); + } catch (err) { + appendLog(`auto-merge failed: cannot open promotion PR: ${err.message}`); + return false; + } + } else { + appendLog(`reusing open promotion PR #${promotionPr.number}`); + } + + const index = promotionPr?.number; + if (!index) { + appendLog("auto-merge failed: promotion PR has no number"); + return false; + } + + // Gitea computes mergeability asynchronously and a large divergence can + // take a while, so poll with backoff instead of giving up after a few + // seconds. `null`/`undefined` means "not computed yet"; only an explicit + // false is a real conflict. + let pr = promotionPr; + let delayMs = 1000; + for (let attempt = 0; attempt < 8; attempt += 1) { + if (pr.mergeable === true || pr.mergeable === false) break; + await new Promise((r) => setTimeout(r, delayMs)); + delayMs = Math.min(delayMs * 1.6, 8000); + pr = await client.getPullRequest(repo.owner, repo.name, index).catch(() => pr); + } + if (pr.mergeable === false) { + appendLog(`auto-merge skipped: promotion PR #${index} conflicts with ${target}`); + await client.createIssueComment(repo.owner, repo.name, index, + `自动合并失败:\`${job.ref_name}\` 与 \`${target}\` 存在冲突,需要人工解决后重新推送。\n\n` + + `审查提交:\`${toSha}\``).catch(() => {}); + return false; + } + if (pr.mergeable === undefined || pr.mergeable === null) { + appendLog(`auto-merge skipped: promotion PR #${index} mergeability still unknown after polling`); + await client.createIssueComment(repo.owner, repo.name, index, + `自动合并暂缓:Gitea 尚未算出该 PR 是否可合并。稍后重新推送或在界面手动合并。\n\n` + + `审查提交:\`${toSha}\``).catch(() => {}); + return false; + } + if (pr.head?.sha && pr.head.sha !== toSha) { + appendLog(`auto-merge skipped: promotion PR #${index} head moved`); + return false; + } + + try { + // Promotion always merges (never squashes): squashing rewrites the + // promoted commits into a brand-new commit, so the two long-lived + // branches diverge a little more on every promotion and eventually + // conflict. A real merge keeps the shared ancestry, so the next + // promotion only carries the new commits. + await client.mergePullRequest(repo.owner, repo.name, index, { + style: "merge", + title: pr.title, + deleteBranch: false, + headCommitId: toSha, + mergeWhenChecksSucceed: repo.auto_merge_mode === "when_checks_succeed", + }); + appendLog(`auto-merge requested for promotion PR #${index}`); + await this.closeRepoIssues({ + client, repo, appendLog, + reason: `${job.ref_name} 已合并到 ${target}(PR #${index})`, + }); + return true; + } catch (err) { + appendLog(`auto-merge failed: promotion PR #${index}: ${err.message}`); + return false; + } + } + + /** + * Close every open review issue for this repository. + * + * Called whenever a merge lands: once code is on its way into the managed + * branch the previous round is over, and any remaining problem will be + * reported again by the next review. + */ + async closeRepoIssues({ client, repo, appendLog, reason }) { + let open = []; + try { + open = await client.get(`/api/v1/repos/${repo.owner}/${repo.name}/issues`, { + query: { state: "open", type: "issues", limit: 100 }, + }) ?? []; + } catch (err) { + appendLog(`cannot list issues to close: ${err.message}`); + return 0; + } + const mine = open.filter((i) => String(i.body || "").includes(SUMMARY_MARKER)); + let closed = 0; + for (const issue of mine) { + try { + await client.createIssueComment(repo.owner, repo.name, issue.number, + `已合并:${reason}。本轮问题视为结束,关闭该 Issue;如后续审查再发现问题会重新创建。`); + await client.updateIssue(repo.owner, repo.name, issue.number, { state: "closed" }); + closed += 1; + } catch (err) { + appendLog(`cannot close issue #${issue.number}: ${err.message}`); + } + } + if (closed) appendLog(`closed ${closed} review issue(s) after merge`); + return closed; + } + async failJob(job, err) { const message = err instanceof SkipJob ? `skipped: ${err.reason}` diff --git a/app/server.js b/app/server.js index aaf2577..7f5b4c5 100644 --- a/app/server.js +++ b/app/server.js @@ -15,7 +15,10 @@ import { import { GiteaClient } from "./lib/gitea.js"; import { OcrRunner } from "./lib/ocr.js"; import { JobQueue } from "./lib/queue.js"; -import { ReviewEngine, SkipJob, branchMatches, parseRepoUrl, pullRequestMatches } from "./lib/review.js"; +import { + ReviewEngine, SkipJob, branchMatches, parseRepoUrl, pullRequestMatches, + PROMOTION_MARKER, +} from "./lib/review.js"; const APP_DIR = resolve(fileURLToPath(new URL(".", import.meta.url))); const ROOT_DIR = resolve(APP_DIR, ".."); @@ -189,6 +192,33 @@ async function handlePush(payload, cfg) { const toSha = payload.after || payload.head_commit?.id; if (!toSha) return { queued: 0, reason: "no head commit in payload" }; + // A merge that a human performed in Gitea also ends the current round: the + // code has landed, so the open review issues are stale. + if (refName === (repo.managed_branch || repo.base_branch) && payload.commits?.length) { + const workspace = join(CONFIG.dataDir, "workspaces", `${repo.owner}__${repo.name}`); + if (existsSync(join(workspace, ".git"))) { + try { + const runner = new OcrRunner({ command: cfg.ocrCommand }); + const parents = await runner.parentCount(workspace, toSha); + if (parents > 1) { + const client = new GiteaClient({ + baseUrl: cfg.giteaUrl, + token: repo.gitea_token || cfg.giteaToken, + }); + const engine = new ReviewEngine({ db, config: cfg, logger }); + const closed = await engine.closeRepoIssues({ + client, repo, appendLog: () => {}, + reason: `管理分支 ${refName} 收到合并提交 ${toSha.slice(0, 10)}`, + }); + if (closed) logger.info(`closed ${closed} issue(s) after merge into ${refName}`); + return { queued: 0, reason: `merge detected on ${refName}; closed ${closed} issue(s)` }; + } + } catch (err) { + logger.warn(`merge detection failed on ${owner}/${name}: ${err.message}`); + } + } + } + const scoped = repo.review_scope === "pr"; const pr = scoped ? null : await findOpenPullRequestForRef(cfg, repo, refName, toSha); if (scoped && !pr) { @@ -224,19 +254,44 @@ const PR_ACTIONS = new Set([ async function handlePullRequest(payload, cfg) { const action = payload.action; - if (!PR_ACTIONS.has(action)) { - return { queued: 0, reason: `action ${action} ignored` }; - } const owner = payload.repository?.owner?.username || payload.repository?.owner?.login; const name = payload.repository?.name; const repo = owner && name ? findRepository(db, owner, name) : null; if (!repo || !repo.enabled) return { queued: 0, reason: "repository not configured" }; + + // A merged pull request is the authoritative "code has landed" signal: the + // open review issues describe a state that no longer exists. + if (action === "closed" && payload.pull_request?.merged) { + const pr = payload.pull_request; + const client = new GiteaClient({ + baseUrl: cfg.giteaUrl, + token: repo.gitea_token || cfg.giteaToken, + }); + const engine = new ReviewEngine({ db, config: cfg, logger }); + const closed = await engine.closeRepoIssues({ + client, repo, appendLog: () => {}, + reason: `PR #${pr.number} 已合并到 ${pr.base?.ref ?? "目标分支"}`, + }); + logger.info(`PR #${pr.number} merged into ${pr.base?.ref}; closed ${closed} issue(s)`); + return { queued: 0, closedIssues: closed, reason: "pull request merged" }; + } + + if (!PR_ACTIONS.has(action)) { + return { queued: 0, reason: `action ${action} ignored` }; + } if (repo.review_scope === "push") { return { queued: 0, reason: "review_scope=push; PRs reviewed via push events" }; } const pr = payload.pull_request; if (!pr) return { queued: 0, reason: "no pull_request in payload" }; if (pr.draft) return { queued: 0, reason: "draft pull request" }; + + // A promotion pull request this service opened itself: the same commits were + // already reviewed when they were pushed, so reviewing again would produce a + // duplicate issue for code that has already been merged. + if (String(pr.body || "").includes(PROMOTION_MARKER)) { + return { queued: 0, reason: "self-created promotion pull request; already reviewed on push" }; + } if (!pullRequestMatches(pr.head?.ref, pr.base?.ref, repo.check_branches)) { return { queued: 0, diff --git a/tests/core.test.js b/tests/core.test.js index e4924af..4cb9d47 100644 --- a/tests/core.test.js +++ b/tests/core.test.js @@ -7,7 +7,7 @@ import { test } from "node:test"; import { rmSync } from "node:fs"; import { parseUnifiedDiff, addedLineNumbers, diffLineNumbers, pickAnchorLine } from "../app/lib/diff.js"; -import { branchMatches, parseRepoUrl } from "../app/lib/review.js"; +import { branchMatches, parseRepoUrl, pullRequestMatches } from "../app/lib/review.js"; import { severityAtLeast, parseList } from "../app/lib/ocr.js"; import { normalizeBaseUrl } from "../app/lib/gitea.js"; import { @@ -100,6 +100,16 @@ test("branchMatches matches an explicit branch list, not globs", () => { assert.ok(!branchMatches("release/2.0", "release/*"), "globs are no longer patterns"); }); +test("pullRequestMatches keys on the target branch, not the feature branch", () => { + // The two-stage flow: feature -> test -> prd. + assert.ok(pullRequestMatches("fix/x", "test", "test,prd"), "PR into test"); + assert.ok(pullRequestMatches("test", "prd", "test,prd"), "promotion PR into prd"); + assert.ok(!pullRequestMatches("fix/x", "master", "test,prd"), "PR into an unprotected branch"); + assert.ok(pullRequestMatches("any", "any", ""), "empty list reviews everything"); + // Listing a long-lived branch still reviews PRs originating from it. + assert.ok(pullRequestMatches("test", "unprotected", "test,prd"), "head branch listed"); +}); + test("parseRepoUrl derives owner and name from every supported URL form", () => { const cases = [ ["http://192.168.31.51/kgod/myrepo.git", "kgod", "myrepo"],