boardx / boardx/workspacex

fix(harness): pr-queue.ts 同款设计——要求 GitHub 原生 APPROVE,本仓实测从未产生过这个信号

Open
#1,441 0 comments 0 reactions 0 assignees View on GitHub
area:harness
Dominant language
TypeScript
Stars
0
Forks
0
Avg merge
1h 7m
Merged PRs (30d)
969

Description

## 背景

复审 `merge-gate.ts`(#956/PR 待定)时发现:条件"独立 approve"原本只接受 GitHub 原生 APPROVE review,但本仓最近 100 个已合并 PR 实测 **0 个**有原生 APPROVE,全仓也搜不到任何脚本调用过 `gh pr review`。已经把 `merge-gate.ts` 改成"原生 APPROVE 或 `review:*-ok` 标签,二者取一"。

**`pr-queue.ts` 的 `classifyPr` 有同款设计**(`.harness/scripts/lib/pr-queue.ts` 「5. review 锚定 SHA」段落):

```ts
const approvals = facts.formalReviews.filter((r) => r.state.toUpperCase() === "APPROVED");
...
if (okLabels.length > 0 && currentShaApprovals.length === 0) {
blocked.push(`verdict label ${okLabels.join("/")} 没有锚定当前 head ... 的独立 approve 背书`);
}
```

同一个"要求原生 APPROVE"的判据,同一个"本仓实际不产生这个信号"的问题。

## 为什么要单独立案,不是顺手在 merge-gate 那个 PR 里一起改

- `pr-queue.ts` 是更早、影响面更大的模块(`classifyPr` 是完整状态机,牵涉 MERGE_BLOCKED/WAITING_REVIEW/CHANGES_REQUIRED 等多态,不是 merge-gate 那种"通过/不通过"二元判定),改动需要更仔细过一遍 31 个既有反证用例,不适合塞进已经聚焦"merge-gate 条件 3"的那个 PR。
- 一 issue 一 PR,范围纪律。

## 风险:两个模块暂时会给出矛盾结论

在本 issue 修完之前,同一个 PR 可能被 `merge-gate`(已改)判"满足独立 approve"(有标签),却被 `pr-queue`(未改)判"没有锚定当前 head 的独立 approve 背书"(同样只认标签不认原生 review,会拦)。这个窗口期应该尽量短。

## 修法

比照 `merge-gate.ts` 的改法:`review:*-ok` 标签可以单独满足"独立 approve"这一条,不再强制要求同时有原生 APPROVE review。复用 `merge-gate.ts` 已经从 `pr-queue.ts` 导出的 `isOkVerdict`(不要在第三处再定义一遍"什么算 OK verdict")。

同样要显式记录代价:标签没有快照 SHA 概念,接受标签路径就意味着放弃"绑定当前 head"这层保护;标签本身也能被任何有写权限的人/agent 自己打。这是已经做过的权衡(见 merge-gate.ts 的同款说明),不是这个 issue 要重新论证的东西,直接抄一致的结论即可。

## 验收

- [ ] `classifyPr` 的"独立 approve"判据改为"原生 APPROVE 或 review:*-ok 标签,二者取一"
- [ ] 既有 31 个用例过一遍,受影响的(涉及自审/漂移场景)比照 merge-gate.test.ts 的做法显式隔离 labels
- [ ] 新增反证:只有标签无原生 review 也应该放行到 READY_TO_MERGE(前提其余条件都满足)
- [ ] 端到端 dogfood:找一个真实 PR 核实 `pr-queue` 与 `merge-gate` 此后给出一致结论

关联:#956、merge-gate.ts、PR(待关联)

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in .harness/scripts/lib/pr-queue.ts at classifyPr and its “review anchored SHA” logic, then compare the established change in merge-gate.ts and its tests. Review the 31 existing counterexample cases, add coverage for a label-only approval reaching READY_TO_MERGE, and verify a real PR gives pr-queue and merge-gate consistent results.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
ci-cd, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.