Skip to content

fix(pr-review): select latest check attempts - #4404

Merged
huangruiteng merged 1 commit into
mainfrom
codex/pr-review-latest-check-attempt
Sep 14, 2026
Merged

huangruiteng merged 1 commit into
mainfrom
codex/pr-review-latest-check-attempt

Conversation

@huangruiteng

Copy link
Copy Markdown
Collaborator

动机 / Motivation

同一 exact head 在 draft→ready、rerun 或 concurrency replacement 后,GitHub 可能同时返回旧 attempt 与最新 attempt。现有 merge-readiness 把所有记录累计,导致已被最新成功结果替代的旧 CANCELLED/FAILURE 永久阻断合并。

GitHub may return superseded and current check-runs together for one exact head. Counting every row makes an obsolete cancelled/failed attempt block merge-readiness even after the current attempt succeeds.

Closes #4403.

改动 / Changes

  • 按 __typename + workflowName + job name 识别 GitHub Actions logical check;对外部 status context 仅在 app/context 和时间均可靠时归一。
  • 只移除开始时间严格早于同一 logical check 最新 attempt 的记录;无身份、无时间或同时间歧义项全部保留,继续 fail closed。
  • checks receipt 新增 raw_total 与 superseded,同时保留 total 作为有效检查数。
  • 将归一逻辑放入 PR-review capability 模块,避免继续扩大已有大型 facade。

产品入口 / Product entry points

  • CLI / managed Turn:loopx pr-review --check-merge-readiness NUMBER@HEAD 直接获得修复后的 typed receipt。
  • Frontend / Lark:无需改动;本修复只影响维护者本地只读合并门禁,没有用户卡片或交互 schema 变化。

验证 / Validation

  • 31 passed:完整 tests/test_pr_review_github_scan.py。
  • 真实 feat(finance): build simulated approval requests #4357:43 个 raw rows → 24 个 effective checks、19 个 superseded;24/24 success,readiness 通过。
  • 反向覆盖:latest pending 继续阻断;跨 workflow 同名 job 保留;无时间歧义行保守保留。
  • Ruff、git diff --check、maintainability ratchet 通过。
  • Standard premerge canary:14 selected checks 全部通过,0 failures,public boundary clean。

风险 / Risk

误归并是主要风险,因此 identity 同时包含 workflow 与 job,并要求可靠 start time;无法证明 superseded 的行不会被删除。变更不放宽 required checks、不授予 merge 权限,也不替代 exact-head review。

Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval conclusion (author-owned PR; GitHub blocks formal self-approval)

Exact head reviewed: 3d147115885a204b83876adb965495eef1645f09

动机

同一 exact head 在 draft→ready、rerun 或 concurrency replacement 后,GitHub 会同时返回旧 attempt 与当前 attempt。原门禁将所有行累计,导致当前 workflow 全绿时仍被旧 CANCELLED/FAILURE 永久阻断;真实 #4357 即为 43 行中混入 19 行已替代结果。

改动思路

在既有 _check_state 与 merge-readiness 之前增加纯归一层:只有 type + workflow + job 身份相同且开始时间严格更早的行才能被判定 superseded。缺身份、缺时间、同时间并列,以及外部 status context 无可靠 app/context 的情况一律保留,延续 fail-closed。门禁仍然不授予 merge authority。

具体改动

  • 新增 capability-owned check_attempts.py,避免扩大 pr_review.py facade。
  • _checks 先选最新 attempt,再沿用原状态分类与 blocker 逻辑。
  • receipt 增加 raw_total、superseded;total 明确为有效当前检查数。
  • README 记录可靠归一和歧义保留语义。
  • 两个回归测试覆盖旧失败→新成功、旧成功→新 pending、跨 workflow 同名 job、无时间歧义行。

关键代码讲解

latest_check_attempts 先为每个可靠 composite identity 求最大 start time,再只删除严格更早的 attempt;并列最大值全部保留。_checks 对返回的 effective rows 使用原 _check_state,因此当前 pending/failure/unknown 的阻断规则没有放宽,新增计数字段让审计者可还原筛选规模。

对主干的风险

主要风险是误把两个真实 required check 合并。实现通过 workflow+job 复合身份、可靠时间要求和歧义保留降低风险;不同 workflow 的同名 job 不合并。单 attempt PR 的行为不变。GitHub Enterprise 若字段不同,只会回到保守阻断。无存储、网络写入、权限或产品交互变化。

我的整体评价

这是修复真实门禁死锁的最小通用改动。它没有用忽略 CANCELLED 或 admin bypass 放宽安全边界,反而同时提高了 current-state 准确性与 receipt 可审计性。结构化审查无发现,建议合并后用其重新核验 #4357。

English verdict: APPROVE. The change removes only provably superseded check attempts, preserves ambiguous and current failing/pending rows, adds auditable counts, and leaves review and merge authority unchanged.

Validation: 31 tests passed; live #4357 normalized 43 raw rows to 24 current successes with 19 superseded; Ruff, diff hygiene, maintainability ratchet, public-boundary scan, and all 14 standard canary checks passed.

@huangruiteng
huangruiteng merged commit 69470a6 into main Sep 14, 2026
47 of 48 checks passed
@huangruiteng
huangruiteng deleted the codex/pr-review-latest-check-attempt branch September 14, 2026 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(pr-review): ignore superseded check attempts in merge readiness

1 participant