Skip to content

fix(pr-review): stop passing statusCheckRollup to gh pr list - #3129

Merged
huangruiteng merged 1 commit into
mainfrom
codex/pr-review-gh-list-fix-20260812
Aug 12, 2026
Merged

fix(pr-review): stop passing statusCheckRollup to gh pr list#3129
huangruiteng merged 1 commit into
mainfrom
codex/pr-review-gh-list-fix-20260812

Conversation

@huangruiteng

@huangruiteng huangruiteng commented Aug 12, 2026

Copy link
Copy Markdown
Owner

What

loopx pr-review failed before scanning because statusCheckRollup was included in the gh pr list --json field set, which the CLI does not support for list (only gh pr view).

Fix:

  • Remove statusCheckRollup from the list fields.
  • Fetch check runs per exact headRefOid via gh api repos/<owner>/<repo>/commits/<head>/check-runs and attach the same rollup shape consumed by the metadata risk hints.
  • A failed lookup leaves the rollup absent (risk hint reports no rollup) instead of failing the whole scan.

Validation

  • pytest tests/test_pr_review_github_scan.py: 2 passed (field exclusion, rollup attachment by exact head, failed-lookup tolerance)
  • python3 examples/pr-review-command-smoke.py: passed
  • Live loopx pr-review --repo huangruiteng/loopx --state open --limit 3: succeeds and attaches checks (e.g., PRs with failing checks now report them)
  • loopx check on changed files: public boundary clean
  • git diff --check: clean

@huangruiteng
fix(pr-review): stop passing statusCheckRollup to gh pr list
6fa8ab4 
gh pr list does not support statusCheckRollup in its --json field set, so
loopx pr-review failed before scanning. Remove the field from the list
command and fetch check runs per exact headRefOid through the commits
check-runs API, attaching the same rollup shape consumed by the risk
hints. A failed lookup leaves the rollup absent instead of failing the
whole scan. Add focused tests for field exclusion, rollup attachment, and
failed-lookup tolerance.
@huangruiteng

huangruiteng commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

Premerge validation summary

Changed surface: loopx pr-review GitHub scan (list fields + per-head check-run fetch) and its focused tests.

Checks run:

  • pytest tests/test_pr_review_github_scan.py: 2 passed
  • python3 examples/pr-review-command-smoke.py: passed
  • Live loopx pr-review --repo huangruiteng/loopx --state open --limit 3: succeeds (previously failed) and attaches check summaries
  • loopx check on changed files: public boundary clean
  • git diff --check: clean
  • LoopX change-quality receipt: cqr_b723de050e5a7424bcec (decision pass)

Failures/skips: none. Manual holds: none.

huangruiteng
huangruiteng commented Aug 12, 2026
View reviewed changes

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Owner 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)

详细中文评审

精确评审头： 3129@6fa8ab45a943237b491b17422a484c84252b02b2

动机

loopx pr-review 的队列扫描在部分 gh 版本/环境上会直接在读取阶段失败：gh pr list 不支持把 statusCheckRollup 放进 --json 字段集（该字段只对 gh pr view 有效），导致整个扫描在进入评审前就报错。本 PR 把该字段从 list 查询中移除，改为按每个 PR 的 exact headRefOid 通过 commits check-runs API 拉取，并保持与 risk hint 消费的 rollup 形状一致；查询失败时容忍为“rollup 缺失”，而不是让整个扫描失败。这是一个真实故障修复，调用点就是 loopx pr-review 生产命令本身。

改动思路

新增 _attach_status_check_rollup(row, repository, cwd)：对每一行按 headRefOid 调用 gh api repos/<owner>/<repo>/commits/<oid>/check-runs --paginate -q .check_runs，把 name/status/conclusion 压缩成与原 view rollup 一致的形状；headRefOid 或 repository 缺失时直接跳过，任何异常或非 list 结果都静默保留“无 rollup”。scan_github_pull_requests 从 gh pr list --json 字段集删除 statusCheckRollup，解析出 api_repository（repo 参数或当前仓库 fallback），并在 detailed.append(row) 之前为每行附加 rollup，保证后续 risk hint 能读到。新增独立测试文件，用 monkeypatch 替换 _run_gh_json，不依赖网络。

具体改动

2 个文件，+148/-1。

关键代码讲解

  • _attach_status_check_rollup：核心是 fail-soft 的“按 exact head 取 check-runs”。异常被整体捕获后返回，行数据保持原样；head_oid/repository 为空时同样安全跳过。这让旧的“整批扫描失败”变成“个别行无 rollup 信息”，是正确方向的降级。
  • scan_github_pull_requests：字段集移除 statusCheckRollup 后，gh pr list 不再因不支持字段报错；api_repository = repo or resolve_current_github_repository(cwd) 让无 --repo 的调用也能解析仓库。attach 调用放在 append 之前，顺序正确。
  • 测试：test_pr_list_json_excludes_status_check_rollup 断言 list 字段不包含该字段、两个 head 的 rollup 被按 exact oid 附加、API 路径正确；test_pr_list_failed_check_lookup_leaves_rollup_absent 断言 API 抛错时扫描仍返回全部行且无 rollup 字段。负向路径覆盖到位。

实测验证（exact head 6fa8ab45a）：直接调用 scan 函数对 3 个 PR 全部成功附加 rollup（#3129 含 7 项、#3128 为空列表、#3120 正确显示 pytest failure）；pytest tests/test_pr_review_github_scan.py 2 passed；examples/pr-review-command-smoke.py 通过；py_compile 通过；原始 gh api .../check-runs --paginate -q .check_runs 返回合法 JSON。

对主干的风险

风险低。改动只落在 pr-review 扫描路径，删除不支持字段修复了真实失败模式；check-runs 按 exact head 拉取，不改变评审语义或权限边界。负向路径上，若 check-runs API 失败（或多页输出导致解析失败），异常都会被捕获并降级为“无 rollup”，扫描继续——信息缺失而非扫描中断，方向正确。两个非阻塞 P2 观察：CLI 的 --format json 输出层不回显已附加的 rollup（risk hint 内部可用，输出层可考虑回显以便读回验证）；check-runs 超过单页时 --paginate -q 的输出拼接可能让 json.loads 失败，最终表现为 rollup 缺失，可接受但值得在后续显式合并分页。GitHub CI 中 pytest 仍在 IN_PROGRESS，其余检查 SUCCESS/SKIPPED；发布前已复核 head 未变。

我的整体评价

APPROVE。 这是有真实调用点的小范围故障修复：scope fit 成立（loopx pr-review 生产命令），删除不支持的字段并改为按 exact head 拉取 check-runs，失败时 fail-soft，测试覆盖正向与负向路径，本地验证全绿。因本 PR 作者与评审账号相同，GitHub 拒绝正式 self-approve，此结论以评论形式记录。

English Verdict

APPROVE — exact head 6fa8ab45a943237b491b17422a484c84252b02b2 (author-owned PR; formal self-approval is blocked by GitHub, so this conclusion is recorded as a comment).

Real bug fix: removes statusCheckRollup from the unsupported gh pr list --json field set and attaches check runs per exact headRefOid via the commits check-runs API with fail-soft behavior. Verified at the exact head: scan-level rollup attachment works live, 2 focused pytest tests, the PR-review command smoke, and py_compile all pass; no blockers. P2 only: the CLI JSON output does not echo the attached rollup, and multi-page check-runs may degrade to an absent rollup (fail-soft, acceptable).

@huangruiteng

huangruiteng commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

CI note: the pytest failures are the same 10 pre-existing model-tool tests from merged #3109 that also fail on main (main run 31574557260); this PR's focused tests pass and build/dependency-review are green. Merging per owner authorization.

@huangruiteng
huangruiteng merged commit 938915c into main Aug 12, 2026
4 of 5 checks passed
@huangruiteng
huangruiteng deleted the codex/pr-review-gh-list-fix-20260812 branch August 12, 2026 11:19
liubf21 pushed a commit to liubf21/loopx that referenced this pull request Aug 14, 2026
@huangruiteng
fix(pr-review): stop passing statusCheckRollup to gh pr list (huangru…
bab2e2d 
…iteng#3129)

gh pr list does not support statusCheckRollup in its --json field set, so
loopx pr-review failed before scanning. Remove the field from the list
command and fetch check runs per exact headRefOid through the commits
check-runs API, attaching the same rollup shape consumed by the risk
hints. A failed lookup leaves the rollup absent instead of failing the
whole scan. Add focused tests for field exclusion, rollup attachment, and
failed-lookup tolerance.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@huangruiteng